Page MenuHomePhabricator

Elm Popup with scrollable enabled is borken
Closed, ResolvedPublic

Description

Open elementary_test go to popup, check enable popup scroll and then click to open some of the popups. The sizing is broken on all of them.

stephenmhouston triaged this task as Showstopper Issues priority.

Oh, this is the same problem I reported with enlightenment connection manager with T6593.

bu5hm4n moved this task from Backlog to widgets on the efl board.Jun 10 2018, 12:48 PM
zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:50 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 7:25 AM

I just spent 8 hours bisecting this. The first bad commit is: 253430ab76348d99c81e3b753b6a32817bdb6666 ... looks like a @bu5hm4n commit. And seems like it would be highly unrelated.

stephenmhouston added a comment.EditedJun 21 2018, 10:37 PM

I can confirm D6361 fixes this. As I am not hip to all of your focus work @bu5hm4n I'm aware this may not be the best fix. You can comment on the patch and lead me in the right direction if you don't have time to fix it, or if its just easier for you to fix it that works too. Thanks!

Hmm.. Actually D6361 only fixes elm_test scrollable popup case and does not fix Ephoto. Interesting.

bu5hm4n added a comment.EditedJun 22 2018, 1:10 AM

The problem i see with this is, that it plainly causes bugs. What you patches does it the following:

Imagine something like

     (a)
    /   \
  /       \
(b)      (c)

Now imagine this little tree n times, where a is connected to b, so we get some sort of cascade.

Currently:

a1->b1->c1->a2->b2->c2->a3->b3->c3 (etc.)

After your patch

a1->b1->a2->b2->a3->b3 (etc.)

Which is definitly not what we should do.

Also, it strikes me quite odd, what causes edje or something else to misplace the scroller, i somehow cannot believe that this is caused by something in focus.

I dont disagree with you. When I found this to be the commit in question I thought I must have accidentally bisected wrong, until I tested the fix and confirmed that fixed it. Something with the focus tree calc realizes all the parts/objects in the scrollable popup?

Okay: progress!!

After a evening of debugging with @okra, (which was quite usefull):

The following patch makes the issue not completly go away but better:
https://git.enlightenment.org/core/efl.git/commit/?id=c2dd97b3c1d241244628f9d53ee1865b9c3e774e

However we have progress!

And my interpretation is, that edje has a bug here, and does not mark itself as smart_changed, but it should.

It is a drawing/rendering issue. See this video: http://www.smhouston.us/stuff/popup_render.ogv ... Notice the focus calcs the objects at the exact coordinates and size and placement of where they are supposed to be, regardless of the popup being drawn incorrectly. For reference of what the popup actually looks like drawn correctly so you can compare to the focus highlight in the video: https://www.enlightenment.org/ss/display.php?image=e-5b3bad1d204ad8.08297482.jpg

Adding evas_render(evas_object_evas_get(sd->spacer)); to line 439 in elc_popup.c fixes all of this. Now the question is why.

zmike added a comment.Jul 3 2018, 2:41 PM

Most likely due to forcing a canvas recalc; see evas_render_pre where pending objects are finalized (unlikely) or evas_smart_objects_calculate which forces a canvas-wide recalc of smart objects which have the recalc flag set. I would expect that by the time the RENDER_PRE callback finishes then whatever is not currently sizing correctly has been fixed based on the above comment.

https://phab.enlightenment.org/D6509 <- is this too expensive? it seems to work.