Skip to content

Conversation

@Meorge
Copy link
Contributor

@Meorge Meorge commented Nov 10, 2025

Closes godotengine/godot-proposals#13609.

This PR allows img tags in RichTextLabels to specify height and width with the em suffix, which makes the values a percentage of the surrounding text's font size.

For example, the following text:

Do you have any [img height=1em]coin.png[/img] coins?
...I said, [font_size=50]DO YOU HAVE ANY [img height=1em]coin.png[/img] COINS??[/font_size]

Displays like this:
CleanShot 2025-11-10 at 10 07 05@2x

@Meorge Meorge changed the title Basic prototype working for "em" units, as percentage of current font size Add option to scale images in RichTextLabel relative to font size Nov 10, 2025
@AThousandShips AThousandShips added this to the 4.x milestone Nov 10, 2025
void add_hr(int p_width = 90, int p_height = 2, const Color &p_color = Color(1.0, 1.0, 1.0), HorizontalAlignment p_alignment = HORIZONTAL_ALIGNMENT_LEFT, bool p_width_in_percent = true, bool p_height_in_percent = false);
void add_image(const Ref<Texture2D> &p_image, int p_width = 0, int p_height = 0, const Color &p_color = Color(1.0, 1.0, 1.0), InlineAlignment p_alignment = INLINE_ALIGNMENT_CENTER, const Rect2 &p_region = Rect2(), const Variant &p_key = Variant(), bool p_pad = false, const String &p_tooltip = String(), bool p_width_in_percent = false, bool p_height_in_percent = false, const String &p_alt_text = String());
void update_image(const Variant &p_key, BitField<ImageUpdateMask> p_mask, const Ref<Texture2D> &p_image, int p_width = 0, int p_height = 0, const Color &p_color = Color(1.0, 1.0, 1.0), InlineAlignment p_alignment = INLINE_ALIGNMENT_CENTER, const Rect2 &p_region = Rect2(), bool p_pad = false, const String &p_tooltip = String(), bool p_width_in_percent = false, bool p_height_in_percent = false);
void add_image(const Ref<Texture2D> &p_image, int p_width = 0, int p_height = 0, const Color &p_color = Color(1.0, 1.0, 1.0), InlineAlignment p_alignment = INLINE_ALIGNMENT_CENTER, const Rect2 &p_region = Rect2(), const Variant &p_key = Variant(), bool p_pad = false, const String &p_tooltip = String(), bool p_width_in_percent = false, bool p_height_in_percent = false, bool p_width_in_em = false, bool p_height_in_em = false, const String &p_alt_text = String());
Copy link
Member

Choose a reason for hiding this comment

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

New args should be at the end to keep script code compatibility, but in this case it's probably better to replace it with enums, something like:

enum ImageUnit {
	IMAGE_UNIT_PIXEL;
	IMAGE_UNIT_PERCENT;
	IMAGE_UNIT_EM;
};

...

void add_image(const Ref<Texture2D> &p_image, int p_width = 0, int p_height = 0, const Color &p_color = Color(1.0, 1.0, 1.0), InlineAlignment p_alignment = INLINE_ALIGNMENT_CENTER, const Rect2 &p_region = Rect2(), const Variant &p_key = Variant(), bool p_pad = false, const String &p_tooltip = String(), ImageUnit p_width_unit = IMAGE_UNIT_PIXEL, ImageUnit p_height_unit = IMAGE_UNIT_PIXEL, const String &p_alt_text = String());

Binary compatibility is preserved by compat bind, and it's rarely used method, so I think it's justified to break script code compatibility a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! This is what I was thinking would ultimately make the most sense for the API, but the compatibility issues had me worried 😅 I'll give this a shot, though!

@Meorge Meorge force-pushed the feat/richtextlabel-img-em branch from 43c28e8 to 4f9ed1a Compare November 10, 2025 22:21
@Calinou
Copy link
Member

Calinou commented Nov 11, 2025

with the em suffix

I'm curious if we should go with rem (root em) instead. Last time I checked, many people considered em in CSS to be a design flaw, which is why most people use rem in CSS nowadays. There are still a few legitimate reasons to use em in CSS in 2025, but rem is more suited in most cases.

That said, maybe it's not a relevant distinction here.

@Meorge Meorge marked this pull request as ready for review November 11, 2025 01:34
@Meorge Meorge requested review from a team as code owners November 11, 2025 01:34
@Meorge
Copy link
Contributor Author

Meorge commented Nov 11, 2025

If I'm understanding rem correctly, it sounds like it wouldn't respect [font_size] BBCode blocks. That is to say, if you had the text

I said, [font_size=50]DO YOU HAVE ANY [img height=100rem]coin.png[/img] COINS??[/font_size]

and the theme/default font size for this RichTextLabel is 16px, then the coin.png sprite would be 16px tall, even though it's in a block of text that's 50px tall.

I suppose we could include both rem and em; the latter would respect [font_size] whereas the former wouldn't. I don't know how many use cases there would be for that, though.

@Meorge Meorge force-pushed the feat/richtextlabel-img-em branch from 9b68a8b to 18054ba Compare November 12, 2025 05:30
@Meorge
Copy link
Contributor Author

Meorge commented Nov 12, 2025

The 1em measurement is equal to 100% of the surrounding font size, rather than 100em.

This required changing the width and height values from ints to floats, so now for free the pixel sizes and percentages also support float values!

Use base font size

Added docs and compatibility methods

Fix compatibility methods maybe for real this time????

Please?

Make width/height use floats instead of ints, and then make 1em be the height of surrounding text

Fix documentation for floats instead of ints

Update compatibility

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

# Conflicts:
#	misc/extension_api_validation/4.5-stable.expected

Add compatibility breakages file
@Meorge Meorge force-pushed the feat/richtextlabel-img-em branch from a157217 to 2e4a0c7 Compare December 5, 2025 17:46
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.

Allow images in RichTextLabel to be sized relative to font size

4 participants