Help wanted: test a subtle text baseline offset / layout fix

[ocornut]

I pushed a test branch:
https://github.com/ocornut/imgui/tree/features/baseline_multiline_label_fixes
With one commit:
0ad1d97

And would love it if some people could test their advanced imgui app with this commit and test me if anything looks off. Specifically, the fix relates the vertical alignment of multiple items of varying size on a same line, and more specically would be more likely to happen when using multi-line label or when altering FramePadding.

If you have time to test this a little, just write a note below (bonus: with a screenshot of what you have tested ;). Thanks!

To quick test you can either update to this branch, either merge the branch in yours, either cherry-pick this commit

git fetch https://github.com/ocornut/imgui features/baseline_multiline_label_fixes
(or git fetch origin features/baseline_multiline_label_fixes)

Followed by
git cherry-pick 0ad1d97fb55fb8472daf4ca5dcfaed874f56758d

Fixed a couple of subtle bounding box vertical positioning issues relating to the handling of text baseline alignment. The issue would generally manifest when laying out multiple items on a same line, with varying heights and text baseline offsets.
Some specific examples, e.g. a button with regular frame padding followed by another item with a multi-line label and no frame padding, such as: multi-line text, small button, tree node item, etc. The second item was correctly offset to match text baseline, and would interact/display correctly, but it wouldn't push the contents area boundary low enough.

My automated test case, before and after:

• The red box is the bounding box reported by the item. It was wrong/off in a few cases in particular when using a multi-line label (BulletText had it too wide, multi-line TreeNode had it off when the treenode came after a framed item).
• The faint white/yellow line between item is the value of CursorPosMax.y (from which the contents size / automatic size is derived) after the item submission. This had little noticable effects in 99% of the case as you can notice that items below are layed out with the right spacing. It would generally only affect you if that item was the last one in the window. I've been using this CursorPosMax value more consistently for the new tables api and the offset really broke tables in some situation. Especially as new tables are reporting text offset baseline from cell to cell, in order to avoid having to call e.g. AlignTextToFramePadding(), it was more easy to get into a situation where you would submit e.g. a multi-line wrapped text and it would size the table line correctly. This is why I got into fixing this in the first place!

Thanks!

(PS: text baseline offset is an obtuse thing that I don't expect many people understand or have notice in dear imgui !)

[BrutPitt]

Ciao, Omar.
I have tested your branch.
In general I have no noticed differences, the only thing is that now appear a scroll bar, although all widgets are well aligned:

ImGui 1.73 Release	baseline multiline label fixes

I immediately say that the problem is my ImGuIZMOquat widget: without it the scrollbar do not appear.

The Child Window have size:
ImGui::GetFrameHeightWithSpacing()*4+ImGui::GetStyle().ItemSpacing.y,

The "arrow" widget have a square size:
ImGui::GetFrameHeightWithSpacing()*3 - ImGui::GetStyle().ItemSpacing.y
And is well aligned, both up and down.

Internally the widget uses ImGui::InvisibleButton("imguiGizmo", size);, where size is the real/total widget size.
With this new branch, to get the same behavior (without scrollbar), I need to use:
ImGui::InvisibleButton("imguiGizmo", size-ImVec2(0,style.ItemInnerSpacing.y));

I noticed also a different y-alignment with ImageButton

ImGui 1.73 Release	baseline multiline label fixes

With the new branch the next line is further down of ImGui::GetStyle().ItemSpacing.y(?) pixels.

I add also the full screenshots from which I have extrapolated the details.

ImGui 1.73 Release	baseline multiline label fixes

Michele

[ocornut]

Thank you very much Michelle for testing and your detailed report, this is exactly what I was hoping for and I realized some of my mistakes in there.

I have pushed a second commit: e484312 which I believe should fix those issues.
And added some more tests.

Details:
I changed the internal ItemSize() api to default to -1.0f for the text baseline offset, signifying "no offset required" and that is the default. There is the risk that users of the internal ItemSize() api filled in a 0.0f (which was the previous defaut) instead of relying default parameters, which for a widget without label would create the same issue you have, until we refactor ItemSize into a more full-featured ItemLayout() which can move the cursor (to achieve flow/wrap layout).

I wanted to do the ItemLayout thing now-ish but realized it was too much work short-term and wanted to focus back on Tables asap, however if my current change back-fire somehow I may have to go and do the full thing.

Note that the - style.ItemInnerSpacing.y of your workaround technically was -style.FramePadding.y.

Part of me wants to remove the text baseline offset system (the common issue is people doing Text + SameLine + Button and seeing the Text vertically misaligned) but that would mean adding FramePadding on Text and TreeNode elements and thus making every existing UI taller, needing to rely on reducing ItemSpacing instead.

[BrutPitt]

Hi, Omar.
Thank you for detailed reply.

I tested new branch in my glChAoS.P application, and now all works fine: ImageButton and my imGuIZMOquat widget are (well) y-aligned like in 1.73 Release.

Note that the - style.ItemInnerSpacing.y of your workaround technically was -style.FramePadding.y.

Thanks for explanation: I had the doubt that it could be one or the other.

Part of me wants to remove the text baseline offset system (the common issue is people doing Text + SameLine + Button and seeing the Text vertically misaligned) but that would mean adding FramePadding on Text and TreeNode elements and thus making every existing UI taller, needing to rely on reducing ItemSpacing instead.

But isn't this already a ImGui::AlignTextToFramePadding(); job?
I think it's great to have both possibilities: like in my ImageButton image (on previous post), where I used a thinner text line as "header" for the widgets below, while with ImGui::AlignTextToFramePadding();, when it is necessary, I get a bigger y-size as the widgets size.

And (maybe/instead) use a Window flag, which makes this for the whole window, without upsetting the current mechanism?

[ocornut]

I pushed a temporary baseline_multiline_label_fixes_docking branch which include this + docking, if needed by anyone to test this.
https://github.com/ocornut/imgui/tree/features/baseline_multiline_label_fixes_docking

[ocornut]

I tested new branch in my glChAoS.P application, and now all works fine: ImageButton and my imGuIZMOquat widget are (well) y-aligned like in 1.73 Release.

Good to hear, thanks!

But isn't this already a ImGui::AlignTextToFramePadding(); job?
I think it's great to have both possibilities:

The current system is quite error-prone both in the library code side and user's side, who rarely call AlignTextToFramePadding(). If we removed it and used e.g. FramePadding for everything by default blocks of text lines would be taller but there would be less variables to understand. One could push FramePadding=0 to remove that spacing.

[ocornut]

This is now merged in master (I also got a second feedback from @rokups about it).
I am not super happy with that change but it seems to be the less annoying stepping stone because we turn ItemSize into a read-write ItemLayout that will allow for new-line flow and other operations.