Skip to content

Conversation

@DictyosteliumDiscoideum
Copy link
Contributor

When a style with a bottom margin was added to a rich text label node,
the bottom margin would be added to the text but the text would not be
clipped.
This adds a line to subtract the bottom margin from the label height
before checking another line should be drawn.
Fixes #87537. Does not fix the additional issue raised regarding
the right margins.

@DictyosteliumDiscoideum DictyosteliumDiscoideum requested a review from a team as a code owner February 10, 2024 04:29
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master 0e9caa2), it works as expected after fixing the compilation error mentioned in my above review comments.

Before

rtl_bottom_margin_before.mp4

After

rtl_bottom_margin_after.mp4

@akien-mga akien-mga modified the milestones: 4.3, 4.4 Jul 30, 2024
@akien-mga akien-mga changed the title Fix Rich Text Label Bottom Margin Fix RichTextLabel bottom margin Aug 19, 2024
@akien-mga akien-mga changed the title Fix RichTextLabel bottom margin Fix RichTextLabel bottom margin for text clipping Aug 19, 2024
@akien-mga akien-mga requested a review from bruvzg August 19, 2024 12:52
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. There's also the same check for the autowrapped lines, and likely need the same change:

Size2 ctrl_size = get_size();
// Draw text.
for (int line = 0; line < l.text_buf->get_line_count(); line++) {
if (line > 0) {
off.y += theme_cache.line_separation;
}
if (p_ofs.y + off.y >= ctrl_size.height) {

@akien-mga akien-mga merged commit 5212a8c into godotengine:master Aug 29, 2024
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RichTextLabel not respecting the content_margin_bottom value.

5 participants