Checkmark label alignment too low

[humdinger]

This is hrev51022.

The label of checkmarks is a bit too low in relation of the checkbox. Here's a screenshot of the TouchPad prefs with a 24 font size to make it more obvious. I added the red line that goes dead center through the checkmark:

[humdinger]

Checkmark label alignment

[anirudh]

I added the following piece of code to CheckBox.cpp under its Draw function.

labelRect.bottom = (float)(0.2f + be_control_look->DefaultLabelSpacing());
labelRect.top = (float)(0.2f + be_control_look->DefaultLabelSpacing());

The label is now moved few pixels up than the correct position (see attachment). I tried modifying the values, but still results the same. What maybe the problem here?

[pulkomandy]

I wonder how we can compute something that works for all fonts and font sizes. For a font, we know the "baseline", which is usually just below the characters, and we know "ascents" and "descents" height (that is, the size of things that can go above or below the baseline). But, this does not give us a way to find the vertical "center" of the font. What should it be aligned with anyway? Center bar of H? Middle of o or central bar of e? In the middle between baseline and ascents?

[humdinger]

I guess one has to try different formulas and judge what looks best. I would've assumed (ascend + descent) / 2 is the middle, which should be aligned to the height / 2 of the checkbox/popup/textcontrol.

[anirudh]

Fixes #13378

[anirudh]

Label Alignment

[pulkomandy]

How is the center of a 0..1 interval at 0.25?

Also, are you sure that this does not change things elsewhere? (BAlignment can be used in many places, not just checkboxes).

Does the label of checkboxes still align properly with the text and labels in other widgets? for example if you put a checkbox and a BStringItem next to each other?

[anirudh]

Instead of modifying the specific alignment for Checkbox, Radiobutton. I managed to change the Vertical Align value of the label across other panels. Here's the preview after the change.

Do let me know if there has to be any changes, I reviewed almost all panels, it does not seem to affect any other view. Let me know. Thanks.

[axeld]

Changing BAlignment is not acceptable, and certainly not the solution to the problem.

[anirudh]

Those were my thoughts too, that was why I did comment ticket:13378#comment:5, but that doesn't seem to solve the problem either.

Replying to pulkomandy:

How is the center of a 0..1 interval at 0.25?

Also, are you sure that this does not change things elsewhere? (BAlignment can be used in many places, not just checkboxes).

Does the label of checkboxes still align properly with the text and labels in other widgets? for example if you put a checkbox and a BStringItem next to each other?

[anirudh]

Fixes #13378

[waddlesplash]

Patch looks OK to me.

[anirudh]

As axeld told that changing BAlignment was not allowed, I went on working with the respective files (CheckBox and RadioButton in this case), and the result looks fine. Let me know if any other component needs label alignment fixing.

[axeld]

The patch doesn't make any sense to me. Why bottom + 1 - default label space? This incorrectly enlarges the label area outside of the view's bounds, and only creates a somewhat fitting (it's too high IMO) offset.

The code in BControlLook::DrawLabel() (​lines 2071ff) is responsible for computing the label location, and it should be investigated why it doesn't work as expected here, and maybe if there is a case where it actually works as it should. This is a relatively new ticket -- maybe it's only broken with the new Robo fonts?

[humdinger]

Replying to axeld:

This is a relatively new ticket -- maybe it's only broken with the new Robo fonts?

It's "Noto", Axel, "Noto". :) And it's not Noto, i.e. changing to the old DejaVu, or any other font for that matter, shows the same slight vertical misalignment.

[axeld]

Hehe, thanks humdinger :-)

[anirudh]

Replying to axeld:

The code in BControlLook::DrawLabel() (​lines 2071ff) is responsible for computing the label location, and it should be investigated why it doesn't work as expected here, and maybe if there is a case where it actually works as it should. This is a relatively new ticket -- maybe it's only broken with the new Robo fonts?

I looked into that, DrawLabel() describes about the label's rectangle area, right? It draws the area where the label is supposed to be. ​lines 2071ff gets the font height, and draws the rectangle accordingly. These do not speak about the alignment of label within the rectangle. Do they?

Should I look into BLayoutUtils::AlignOnRect(), maybe that deals with the alignment of label inside the rectangle?

The patch doesn't make any sense to me. Why bottom + 1 - default label space? This incorrectly enlarges the label area outside of the view's bounds, and only creates a somewhat fitting (it's too high IMO) offset.

What I presumed was, since the DrawLabel() draws the rectangle, creating a padding for the label's bottom will push it to the center.

Replying to humdinger:

changing to the old DejaVu, or any other font for that matter, shows the same slight vertical misalignment.

Did my patch fix it for the other fonts, or it fixes only for Noto?

[axeld]

While your change might have fixed the symptoms in the default configuration, it surely doesn't get to the bottom of the problem, and thus, doesn't fix the actual issue. To fix a problem for real, one has to fully understand it first.

The code in BControlLook::DrawLabel() does look correct to me, as does how it's being called from BCheckBox, and BRadioButton. However, there must be a problem, or else it wouldn't show on screen, wouldn't it?

[pulkomandy]

Marked patches as obsolete as they don't handle the problem the right way.