Page MenuHomePhabricator

ui.box: remove leagcy evas_box from Efl.Ui.Box
ClosedPublic

Authored by YOhoho on Mar 19 2019, 9:54 PM.

Details

Summary

Remove legacy stuff from Efl.Ui.Box.
This expect to improve performance by removing internal function call related
evas_box.

Depends on D8433

Test Plan
  • make check
  • elementary_test -to 'efl.ui.box'
  • elementary_test -to 'efl.ui.box_stack'

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.
YOhoho created this revision.Mar 19 2019, 9:54 PM
YOhoho requested review of this revision.Mar 19 2019, 9:54 PM
YOhoho updated this revision to Diff 20769.Mar 19 2019, 10:00 PM
YOhoho edited the summary of this revision. (Show Details)

edit commit message

YOhoho updated this revision to Diff 20770.Mar 19 2019, 10:04 PM
YOhoho edited the summary of this revision. (Show Details)

.

YOhoho updated this revision to Diff 20771.Mar 19 2019, 10:33 PM

add direction, homogeneous unit test

YOhoho updated this revision to Diff 20775.Mar 20 2019, 1:36 AM

use efl_key_data_set

I am having troubles understanding how Index works. Please, correct me if I am wrong.
Negative indices only mean that you count from last to first, instead of first to last, right?
So, if a container has 4 items, you can access the first one using index 0 or index -4, and you can access the last item using index 3 or index -1. Correct?
In this case, the docs could be clarified a bit.
Also, some methods (like pack_content_get) accept the [-count, count-1] range, whereas other methods accept the [0, count-1] range (like pack_index_get). The docs for each method should clearly state the valid range.

If my guesses are correct, you can create a documentation task and assign it to me :)

YOhoho updated this revision to Diff 20833.EditedMar 20 2019, 9:52 PM
YOhoho edited the summary of this revision. (Show Details)

I am having troubles understanding how Index works. Please, correct me if I am wrong.
Negative indices only mean that you count from last to first, instead of first to last, right?
So, if a container has 4 items, you can access the first one using index 0 or index -4, and you can access the last item using index 3 or index -1. Correct?
In this case, the docs could be clarified a bit.
Also, some methods (like pack_content_get) accept the [-count, count-1] range, whereas other methods accept the [0, count-1] range (like pack_index_get). The docs for each method should clearly state the valid range.

If my guesses are correct, you can create a documentation task and assign it to me :)

That is totally correct. please check documentation revision D8433

YOhoho updated this revision to Diff 20862.Mar 22 2019, 2:32 AM

add efl_pack_unpack_at unit test

zmike added a comment.Mar 27 2019, 8:10 AM

I am thinking that maybe we land this patch after the 1.22 release?

In D8417#153395, @zmike wrote:

I am thinking that maybe we land this patch after the 1.22 release?

Yes :)

segfaultxavi requested changes to this revision.Apr 1 2019, 4:55 AM
segfaultxavi added inline comments.
src/lib/elementary/efl_ui_box.c
279–280

This changes the previous behavior, and is not documented in D8433 (previous patch in this stack).

This revision now requires changes to proceed.Apr 1 2019, 4:55 AM
YOhoho updated this revision to Diff 21121.Apr 1 2019, 8:31 PM

I forgot git add

segfaultxavi resigned from this revision.Apr 2 2019, 1:55 AM

OK, I am happy with the docs now, thanks!
However, I will not comment on the code, since I am no expert in this area.

bu5hm4n requested changes to this revision.Apr 2 2019, 8:46 AM
bu5hm4n added a subscriber: bu5hm4n.
bu5hm4n added inline comments.
src/lib/elementary/efl_ui_box.c
284

count + 1 is wrong,

Box with 3 items. pack_at with -1
Means we have a index of 3.
Means that eina_list_nth_list is NULL.
So -1 prepends it to the beginning instead to the last item.

This revision now requires changes to proceed.Apr 2 2019, 8:46 AM
segfaultxavi requested changes to this revision.Apr 3 2019, 2:54 AM
segfaultxavi added a subscriber: segfaultxavi.
segfaultxavi added inline comments.
src/lib/elementary/efl_ui_box.c
284

I agree with @bu5hm4n !

YOhoho updated this revision to Diff 21207.Apr 3 2019, 3:48 AM

Thank you to point out that.

segfaultxavi accepted this revision.Apr 3 2019, 3:57 AM

All test suites (including the new behavioral one) pass now!

I see nothing else wrong with indices, but somebody else with more knowledge of Widget internals should review this :)

bu5hm4n accepted this revision.Apr 3 2019, 4:53 AM

Cool, thx!

This revision is now accepted and ready to land.Apr 3 2019, 4:53 AM
YOhoho updated this revision to Diff 21335.Apr 12 2019, 3:18 AM
YOhoho removed a project: DO NOT MERGE.

rebase

Closed by commit rEFLd5445918ec9f: ui.box: remove leagcy evas_box from Efl.Ui.Box (authored by Yeongjong Lee <yj34.lee@samsung.com>, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyApr 17 2019, 6:21 AM
This revision was automatically updated to reflect the committed changes.