Page MenuHomePhabricator

efl_ui_item: remove sizing eval code
ClosedPublic

Authored by bu5hm4n on Jul 28 2019, 8:42 AM.

Details

Summary

i do not know why this code is there. But the same code is called in
layout itself, additionally this results in way less calls for
calculating the minsize (Not sure why).

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.Jul 28 2019, 8:42 AM
bu5hm4n requested review of this revision.Jul 28 2019, 8:42 AM

the reason it has sizing eval is there are some demands that skipping sizing eval such as homogeneous size in the list or grid..
if we already know the size, we could skipping restricted size calculation and set the size by it's container.
I didn't write the code for skipping case, and that is why you couldn't understand the existance.

we need to update the item size calc skipping logic with listview position manager on homogeneous model.

I am not understanding what you mean. This is the exactly same code than it is in layout, so for now it's just duplicated code afaics. I am also not entirely sure how you want to realize skipping that call...

I think @zmike has been working recently on this method, he might know more.

zmike added a comment.Jul 29 2019, 5:16 AM

You can shorten this patch to just remove the bool from the struct in the header, I've already reworked everything here in my branch and I'd prefer to not have to rebase again.

SanghyeonLee added a comment.EditedJul 29 2019, 9:54 PM

@bu5hm4n yeah we don't have code to skipping it yet.

so the idea is,
"edje_object_size_min_restricted_calc"
is very expensive call to get the size, especially if you know the size what will be.

so instead of calling this restrict_calc in the sizing_eval on every item,
get the first item's size and caching it in the homogeneous model,
so every new item require the sizing eval,
just set the size by old cached size instead of calling the restrict_calc in the sizing_eval.

at least this is old way of genlist improve their performance in homogeneous mode,
I'm not sure it will be same on the homogeneous model,
but calling restrict_calc per items will be very expensive so I think we should have the code to abandon restrict_calc in the item.
other way is only guessing size will be homogeneous and calculate the whole scroll content size,
but real object realized, get the size eval and correcting right size again..
but this will also bad performance on the first scrolling.

I agree that current code is just copy of layout,
and you could remove this code for now.
I just want to explain why this unnecessary code is existed.

Okay - after @zmike patches this method will be gone since that will be handeled differently, but i think such a optimization can be implemented afterwards, without much trouble ... :)

zmike added a comment.Jul 30 2019, 6:47 AM

@SanghyeonLee I have some ideas for improving this type of thing even further in the future, it would be great if you could add a sample case (using efl_ui_item) that I can use for future profiling.

bu5hm4n updated this revision to Diff 23767.Jul 30 2019, 10:31 AM
bu5hm4n edited the summary of this revision. (Show Details)

remove the leftovers

zmike accepted this revision.Jul 30 2019, 10:47 AM
This revision is now accepted and ready to land.Jul 30 2019, 10:47 AM
Closed by commit rEFL911eab34207f: efl_ui_item: remove sizing eval code (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyJul 30 2019, 11:42 PM
This revision was automatically updated to reflect the committed changes.