uilist line padding accounts for number of lines in text
Summary
Bugfixes "uilist line padding accounts for number of lines in text"
Purpose of change
- Fixes #75698
Describe the solution
Padding is applied between each line ( s.ItemSpacing.y * expected_num_lines ) and then also two at the top and bottom of the text: ( s.ItemSpacing.y * 2.0 ). Previously the calculated padding area only account for the top and bottom, meaning that each additional line of text ate up more and more vertical space that the window wasn't expecting
I'm not sure why ImGui::CalcTextSize() isn't already returning this properly
Describe alternatives you've considered
There is probably somewhere more fundamental to apply this, like the static calc_size
title_size and desc_size are both calculated the previous way, and technically those ~~likely~~ do have the same error unless changes are made to them/calc_size
I am sure there is a better way to do this, I just don't know what it is!
Testing
See my beautiful images at the comment I made in the linked issue
Additional context
I guess we can just mark this ready and see if any objections come up. It shouldn't make the UI any worse!
I suppose we can go ahead and do this. I had been intending to investigate to find out where the extra spacing comes from, since it indicates to me that we are doing something wrong, but I guess that working around it won’t make that any more difficult.
I suppose we can go ahead and do this. I had been intending to investigate to find out where the extra spacing comes from, since it indicates to me that we are doing something wrong, but I guess that working around it won’t make that any more difficult.
Doesn't the extra spacing come from ItemSpacing?
And (separately), to handle this properly (though it is definitely scope creep), I suspect that ItemInnerSpacing, CellPadding, and possibly FramePadding will need to be handled (I think WindowPadding is already handled)
Sure, it apparently comes from ItemSpacing but if it’s supposed to be there why doesn’t CalcTextSize include it?
Sure, it apparently comes from
ItemSpacingbut if it’s supposed to be there why doesn’tCalcTextSizeinclude it?
That's easy, CalcTextSize only calculates the size of the text. See ocornut/imgui/issues/3714
The answer is not so simple. We were calculating the height as he recommends (“CalcTextSize(text).y + style.FramePadding.y * 2.0f for framed items”), but found that we need an additional style.ItemSpacing.y * 2.0f per line of text. If that’s intentional then why doesn’t CalcTextSize include it? I think it must be a bug in our code somewhere.
DB/CLIDragon if you want to reopen the linked issue around to track what's going on with the vertical spacing until you can really nail it, let me or other triage know. Whatever's easier for you to work on it