Opened 3 years ago

Last modified 16 months ago

#13378 assigned enhancement

Checkmark label alignment too low

Reported by: humdinger Owned by: nobody
Priority: normal Milestone: Unscheduled
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

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:

Checkmark label alignment

Attachments (5)

Checkbox_Layout.png (5.2 KB ) - added by humdinger 3 years ago.
Checkmark label alignment
Checkbox_label.png (64.2 KB ) - added by anirudh 3 years ago.
0001-Label-Alignment.-Fixes-13378.patch (693 bytes ) - added by anirudh 3 years ago.
Fixes #13378
Label Alignment.png (22.9 KB ) - added by anirudh 3 years ago.
Label Alignment
0001-Fixes-13378.patch (1.6 KB ) - added by anirudh 3 years ago.
Fixes #13378

Download all attachments as: .zip

Change History (27)

by humdinger, 3 years ago

Attachment: Checkbox_Layout.png added

Checkmark label alignment

comment:1 by humdinger, 3 years ago

Has a Patch: set

comment:2 by humdinger, 3 years ago

Has a Patch: unset

by anirudh, 3 years ago

Attachment: Checkbox_label.png added

comment:3 by anirudh, 3 years ago

Has a Patch: set

comment:4 by anirudh, 3 years ago

Has a Patch: unset

comment:5 by anirudh, 3 years ago

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?

comment:6 by pulkomandy, 3 years ago

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?

comment:7 by humdinger, 3 years ago

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.

by anirudh, 3 years ago

Fixes #13378

comment:8 by anirudh, 3 years ago

Has a Patch: set

by anirudh, 3 years ago

Attachment: Label Alignment.png added

Label Alignment

comment:9 by pulkomandy, 3 years ago

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?

in reply to:  8 comment:10 by anirudh, 3 years ago

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.

https://dev.haiku-os.org/raw-attachment/ticket/13378/Label%20Alignment.png

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.

comment:11 by axeld, 3 years ago

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

in reply to:  9 comment:12 by anirudh, 3 years ago

Those were my thoughts too, that was why I did 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?

Last edited 3 years ago by anirudh (previous) (diff)

comment:13 by axeld, 3 years ago

Owner: changed from axeld to nobody
Status: newassigned

by anirudh, 3 years ago

Attachment: 0001-Fixes-13378.patch added

Fixes #13378

comment:14 by waddlesplash, 3 years ago

Patch looks OK to me.

comment:15 by anirudh, 3 years ago

https://i.imgur.com/DiQktbS.png

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.

comment:16 by axeld, 3 years ago

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?

in reply to:  16 ; comment:17 by humdinger, 3 years ago

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.

comment:18 by axeld, 3 years ago

Hehe, thanks humdinger :-)

in reply to:  17 comment:19 by anirudh, 3 years ago

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?

comment:20 by axeld, 3 years ago

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?

comment:21 by pulkomandy, 16 months ago

Has a Patch: unset

comment:22 by pulkomandy, 16 months ago

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

Note: See TracTickets for help on using tickets.