Page MenuHomePhabricator

ui.box: refactor layout_update
ClosedPublic

Authored by YOhoho on Jan 24 2019, 1:35 AM.

Details

Summary

Current layout_update doesn't consider max hint priority. for example, if it
has hint_max, the size is hint_max value regardless of weight or fill.
moreover, if it has hint_aspect with hint_max, hint_min can be ignored.
(test that aspect(1,3) max(50,150), min(70, 70) then, size has (50, 150))
It seems that hint_max is considered as high priority. however, if hint_min
greater than hint_max, hint_max is ignored.

In order to resolve the hint priority conflict, this refactoring supports
following hint priority.

  1. HintMin
  2. HintMin + HintAspect
  3. HintMargin
  4. HintMax
  5. HintAspect
  6. HintWeight, HintFill
  7. HintAlign

ref T5487

Specific unit test is D7463

Test Plan

make check
elementary_test -to 'efl.ui.box'

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9067
Build 7824: arc lint + arc unit
YOhoho created this revision.Jan 24 2019, 1:35 AM
YOhoho requested review of this revision.Jan 24 2019, 1:35 AM
YOhoho edited the test plan for this revision. (Show Details)Jan 24 2019, 1:37 AM
YOhoho updated this revision to Diff 19214.Feb 6 2019, 7:27 AM

rewrite aspect calculation

Aspect type make race conditon. to resolve this problem, ui_box support only aspect_both type.

YOhoho updated this revision to Diff 19226.Feb 6 2019, 6:33 PM

remove duplicated code

zmike added a subscriber: zmike.Feb 7 2019, 10:47 AM

Can you elaborate a bit on what bugs are intended to be fixed?

zmike requested changes to this revision.Feb 8 2019, 9:13 AM

Really gonna need that explanation in order to review this properly.

This revision now requires changes to proceed.Feb 8 2019, 9:13 AM
YOhoho added a comment.EditedFeb 10 2019, 3:57 PM

@zmike
Commit message is updated.
And one more important thing - it ensures consistency of hint behavior on containers(box, table(D7841), relative layout(D7524)).

YOhoho updated this revision to Diff 19306.Feb 12 2019, 1:31 AM

initialize w,h in for loop.

@segfaultxavi what do you think about adding this hint priority to documentation somewhere?

I think the right place to describe this is in the documentation for the Efl.Gfx.Size_Hint class, in src/lib/efl/interfaces/efl_gfx_size_hint.eo.
Each hint is described on its own, but the priorities could be described in the class docs.
I will take care of this in T7694.

There's a tutorial about UI sizing, but right now it is extremely poor:
https://www.enlightenment.org/develop/guides/c/ui/sizing.md

zmike accepted this revision.Feb 12 2019, 6:55 AM
This revision is now accepted and ready to land.Feb 12 2019, 6:55 AM
YOhoho updated this revision to Diff 19349.Feb 13 2019, 4:50 AM

fix aspect calculation bug

Closed by commit rEFL846492ebd7b8: ui.box: refactor layout_update (authored by Yeongjong Lee <yj34.lee@samsung.com>, committed by zmike). · Explain WhyFeb 13 2019, 6:23 AM
This revision was automatically updated to reflect the committed changes.

This commit broke the examples!
For example:
https://git.enlightenment.org/tools/examples.git/tree/tutorial/c/hello-gui/src/gui_main.c
or
https://git.enlightenment.org/tools/examples.git/tree/apps/c/texteditor/src/texteditor_main.c

These examples contain an Efl.Ui.Box where the hint size min is set, which was previously respected but not anymore. Now the window has the minimum size to accommodate all widgets, instead of the min size set to the box.
Am I using this hint incorrectly?

YOhoho added a comment.Mar 4 2019, 5:37 PM

This commit broke the examples!
For example:
https://git.enlightenment.org/tools/examples.git/tree/tutorial/c/hello-gui/src/gui_main.c
or
https://git.enlightenment.org/tools/examples.git/tree/apps/c/texteditor/src/texteditor_main.c

These examples contain an Efl.Ui.Box where the hint size min is set, which was previously respected but not anymore. Now the window has the minimum size to accommodate all widgets, instead of the min size set to the box.
Am I using this hint incorrectly?

My bad. i didn't consider layout(parent) hint size min. D8098 will fix that issue.