Page MenuHomePhabricator

efl_ui_tab_pager: migrate to active_view
ClosedPublic

Authored by bu5hm4n on May 16 2019, 12:17 PM.

Details

Summary

This now migrates to active_view. When we get back to this widget for
stabelization. This is only the first step for loosening efl.ui.pager so
it can be removed. For now the API of efl_ui_tab_pager stayes the same.

Depends on D8784

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bu5hm4n created this revision.May 16 2019, 12:17 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

bu5hm4n requested review of this revision.May 16 2019, 12:17 PM
bu5hm4n added a child revision: D8907: efl.ui.pager: remove!.
bu5hm4n added a subscriber: zmike.May 17 2019, 12:14 AM

@zmike Can you build this branch Patch, open "elementary_test -to "Efl.Ui.Tab_Pager" click to transition, click on stack, then click a little bit around on the icons, see the wonderful animation, then click on scroll, click around on the icons, see the wonderful animation. Now if you click on efl again, do you also see some artifacts where the font "efl" is flashing around on the same position in the window ? I printed the geometry of the content, and the content is moved totally linear, there is no such jumping around geometry wise. Any ideas what could cause this ?

bu5hm4n updated this revision to Diff 22260.May 19 2019, 4:00 AM
bu5hm4n edited the summary of this revision. (Show Details)

general update

I tested the tab pager example but I found some weird scroll transition when I drag the views.
(selecting tab is ok but if I drag view with scroll transition, then the scroll transition look weird)

Now I am investigating the reason.

I am also investigating this now. That is rather new ...

Jaehyun_Cho requested changes to this revision.May 29 2019, 5:06 AM
This comment was removed by Jaehyun_Cho.
src/lib/elementary/efl_ui_tab_pager.c
29–30

Please check the following conditional expression not to set the same active index again.

if (index != efl_ui_active_view_active_index_get(data))

This revision now requires changes to proceed.May 29 2019, 5:06 AM

It seems that the scroll transition issue happens because the same active index is set again.

I think that the better way to fix is to fix efl_ui_active_view_container_active_index_set to check if the current active index is the same with the given index.

Jaehyun_Cho added inline comments.May 29 2019, 5:18 AM
src/lib/elementary/efl_ui_tab_pager.c
29–30

Please ignore this comment.
As I commented above, fixing efl_ui_active_view_container_active_index_set looks better than this.

Jaehyun_Cho accepted this revision.May 29 2019, 5:19 AM
This revision is now accepted and ready to land.May 29 2019, 5:19 AM

The problem was actaully that i forgot to reset pd->transition.progress in the switch_to call of the view_manager_scroll.c This is now fixed and should work.

The reason i do want to allow the same setting of active_index twice is, that when a view_manager allows user interaction, it is super usefull to just "flush out" the current position rounded to an integer, and let the animation code deal with the rest, this safes a lot of states when you think of the view_manager as a state maschine :)

Jaehyun_Cho requested changes to this revision.May 29 2019, 5:35 AM

I applied the latest D8784 and I tested again.

I found the drag does not work if I change transition from stack to scroll.

Now I am not sure if it is related to this patch or D8784.

This revision now requires changes to proceed.May 29 2019, 5:35 AM

This is a bug where somewhere a rectangle is leaked, but i cannot find out where or why. I will keep searching this, But based on the fact that no evas_object_rectangle_add is executed in this nor the other patches, i would claim that the bug is somewhere else ... will be an additional fix ... sorry for that.

bu5hm4n added a comment.EditedMay 29 2019, 6:08 AM

nvm my previous comment, i found the leak, this is so embarrassing :(

https://phab.enlightenment.org/D8784#165543 fixes the issue

Thank you! :)

Just one more thing about tab pager test case.
When I tested Current Tab Page Set in Efl.Ui.Tab_Pager in elementary_test with Scroll transition, the transition only shows 2 views.
e.g. if now active view: 0, new active view: 2 -> it shows from view 1 to view 2

If I click the tab in the same case, then the transition shows 3 views correctly. (from view 0 to view 2)

I am investing the reason.

Jaehyun_Cho added a comment.EditedMay 29 2019, 11:45 PM

I've found out that the different scroll transition happens because efl_ui_active_view_active_index_set is called twice.
This causes efl_ui_active_view_active_index_set called with the same index value again.
e.g. if now active view: 0, new active view: 2, active view becomes from 0 to 2 and from 2 to 2.

There are 2 solutions.

  1. check current index in _tab_select_cb in efl_ui_tab_pager.c
    • efl_ui_active_view_active_index_set is called only if efl_ui_tab_bar_current_tab_get is not same with efl_ui_active_view_active_index_get
  2. check current index in efl_ui_active_view_active_index_set
    • do nothing if the same index is given in efl_ui_active_view_active_index_set

I think you may want the solution 1.
But I hope we choose solution 2. Because when I tested Efl.Ui.Active_View Scroll in elementary_test, the scroll transition shows differently.
e.g. if now active index: 0 new active index:0,

if I click Set Active Index button one time, no scroll transition
if I double click Set Active Index button (click twice fast), scroll transition happens but the scroll transition is not full animation.

I think that allowing same active index may be more complicated to maintain. So I hope we choose solution 2.

bu5hm4n updated this revision to Diff 22602.May 30 2019, 12:14 AM

only set active_index when there is a real change

Thank you for tracking that down! I decided for solution number 1, i had solution one, but then the animation code from the scroll view_manager is really much, additionally, we would need there a way to allow the same setting for this reason:

  • We are dragging the views with the scroll view_manager arround, when we began, active_index was 2, now we dragged to 0, but something from the UI now wants to immidiatly display active_index = 2. With the filter of not allowing the same active_index twice, we would end up not going back to view 2. With the solution we have right now, we will :)
Jaehyun_Cho accepted this revision.May 30 2019, 1:21 AM

I see~ Thank you :)

This revision is now accepted and ready to land.May 30 2019, 1:21 AM
Closed by commit rEFLbdc63d57d527: efl_ui_tab_pager: migrate to active_view (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyMay 30 2019, 2:48 AM
This revision was automatically updated to reflect the committed changes.