UI: Automatic text contrast for overlays via outline #121383

Aras Pranckevicius · 2024-05-03T09:49:48+02:00

2024-05-03 09:49:48 +02:00

Continuing #121239 (I don't know how to do a PR that builds upon someone else open PR lol). In addition to that PR:

Instead of "shadow level" integer where the only valid options are 0, 3 or 5, have a FontShadowType enum.
Add a new FontShadowType::Outline enum entry, that does a 1px outline by doing a 3x3 dilation in the font shader.
BLF_draw_default_shadowed is changed to do outline, instead of drawing the shadow twice (maybe the name should get changed?)
In the font shader, instead of encoding shadow type in signs of the glyph_size, pass that as a "flags" vertex attribute. Put font texture channel count into the same flags, so that the vertex size stays the same.
Well actually, vertex size becomes smaller by 4 bytes, since turns out glyph_mode vertex attribute was not used for anything at all.

Here's how the outline looks in 3D overlay:

Continuing #121239 (I don't know how to do a PR that builds upon someone else open PR lol). In addition to that PR: - Instead of "shadow level" integer where the only valid options are 0, 3 or 5, have a `FontShadowType` enum. - Add a new `FontShadowType::Outline` enum entry, that does a 1px outline by doing a 3x3 dilation in the font shader. - `BLF_draw_default_shadowed` is changed to do outline, instead of drawing the shadow twice (maybe the name should get changed?) - In the font shader, instead of encoding shadow type in signs of the glyph_size, pass that as a "flags" vertex attribute. Put font texture channel count into the same flags, so that the vertex size stays the same. - Well actually, vertex size becomes smaller by 4 bytes, since turns out `glyph_mode` vertex attribute was not used for anything at all. Here's how the outline looks in 3D overlay: ![image](/attachments/673a6e3f-3a1e-48fd-8d18-b77de49e0a93) ![image](/attachments/8ad8dbfc-78fe-4185-9291-9aad111418e5) ![image](/attachments/2f46c5f2-0a43-4997-8abe-bdfa2a7a6066)

👍 2 ❤️ 8 🎉 2 🚀 2

Aras Pranckevicius added 4 commits 2024-05-03 09:49:55 +02:00

WIP: UI: Automatic Text Contrast for Overlays 9a75b82ec4

Automatic text color and shadows to give contrast for text overlays.

Cleanup: BLF glyph_mode vertex attribute was not used at all 77088a702e

BLF: add "outline" font shadow type, use enum for shadow types 8a302367c4

- Instead of "shadow level" integer where the only valid options are 0,
  3 or 5, have a FontShadowType enum.
- Add a new FontShadowType::Outline enum entry, that does a 1px outline
  by doing a 3x3 dilation in the font shader.
- BLF_draw_default_shadowed is changed to do outline, instead
  of drawing the shadow twice (maybe the name should get changed?)
- In the font shader, instead of encoding shadow type in signs
  of the glyph_size, pass that as a "flags" vertex attribute. Put
  font texture channel count into the same flags, so that the vertex
  size stays the same.

Merge branch 'main' into text-shadow-contrast 07f7b697ed

# Conflicts:
#	source/blender/blenfont/intern/blf_glyph.cc
#	source/blender/blenfont/intern/blf_internal_types.hh

Aras Pranckevicius referenced this pull request

2024-05-03 09:53:22 +02:00

WIP: UI: Automatic Text Contrast for Overlays #121239

Harley Acheson commented

2024-05-04 02:40:40 +02:00

turns out glyph_mode vertex attribute was not used for anything at all

That's a whole thing...

For the longest time we have only supported a single output type and single output bitmap from FreeType. But it supports much more than our current monochrome output using a greyscale coverage map. It (optionally) supports color fonts, SDF, and subpixel rendering (where the physical placement of the LCD pixels is considered).

So I updated our FreeType code to support all of the formats, even though we only use one now, with the format type communicated to the shader in that glyph_mode attribute. Because eventually we not only have to support more formats, but also multiple formats at once. So we might support color (for icons) but would still need to support monochrome for other characters. Or when subpixel rendering there are times when you need to render without it (usually for particular compositing problems, like when animating).

In a nutshell this is useless now but could allow someone who knows shaders to experiment with this stuff without having to also know how FreeType works. For example, in blf_glyph_render_bitmap you can add render_mode = FT_RENDER_MODE_SDF; and you will get SDFs shown. However, if you then alter the shader to render SDF properly you might not be happy. Generally with those you just render at a larger size and use that one bitmap size for a range of sizes below it. So just using a 11px SDF for 11px text isn't quite good enough. But it is a quick test.

Similarly you can just add render_mode = FT_RENDER_MODE_LCD; and you will get the format needed for this - three coverage maps for each of R, G, and B. I have managed to at least get this readable on my own, but not well. This apparently needs to use dual source blending to mix the colors correctly with the background.

But anyway, that is why there is that glyph_mode attribute that is currently not used. No problem getting rid of it for now since we are not in danger of needing it soon. But eventually (hopefully) we will need to differentiate between multiple modes that require different rendering - and might share the same bitmap depth (like mono and SDF, and the two LCD modes).

> turns out glyph_mode vertex attribute was not used for anything at all That's a whole thing... For the longest time we have only supported a single output type and single output bitmap from FreeType. But it supports much more than our current monochrome output using a greyscale coverage map. It (optionally) supports color fonts, SDF, and subpixel rendering (where the physical placement of the LCD pixels is considered). So I updated our FreeType code to support all of the formats, even though we only use one now, with the format type communicated to the shader in that glyph_mode attribute. Because eventually we not only have to support more formats, but also multiple formats at once. So we might support color (for icons) but would still need to support monochrome for other characters. Or when subpixel rendering there are times when you need to render without it (usually for particular compositing problems, like when animating). In a nutshell this is useless now but could allow someone who knows shaders to experiment with this stuff without having to also know how FreeType works. For example, in `blf_glyph_render_bitmap` you can add `render_mode = FT_RENDER_MODE_SDF;` and you will get SDFs shown. However, if you then alter the shader to render SDF properly you might not be happy. Generally with those you just render at a larger size and use that one bitmap size for a range of sizes below it. So just using a 11px SDF for 11px text isn't quite good enough. But it is a quick test. Similarly you can just add `render_mode = FT_RENDER_MODE_LCD;` and you will get the format needed for this - three coverage maps for each of R, G, and B. I have managed to at least get this readable on my own, but not well. This apparently needs to use dual source blending to mix the colors correctly with the background. But anyway, that is why there is that glyph_mode attribute that is currently not used. No problem getting rid of it for now since we are not in danger of needing it soon. But eventually (hopefully) we will need to differentiate between multiple modes that require different rendering - and might share the same bitmap depth (like mono and SDF, and the two LCD modes).

Aras Pranckevicius commented

2024-05-04 07:09:05 +02:00

But anyway, that is why there is that glyph_mode attribute that is currently not used.

I think once/if that is needed, it can be put into the "flags" attribute without making the whole vertex size larger. But also, if some render modes require changing blending state (to enable dual source blending and set blend modes appropriately), then it sounds like it should not be an attribute at all, but rather a uniform / push constant, and the CPU side code would need to put texts using different render modes into different batches anyway.

> But anyway, that is why there is that glyph_mode attribute that is currently not used. I think once/if that is needed, it can be put into the "flags" attribute without making the whole vertex size larger. But also, if some render modes require changing blending state (to enable dual source blending and set blend modes appropriately), then it sounds like it should not be an attribute at all, but rather a uniform / push constant, and the CPU side code would need to put texts using different render modes into different batches anyway.

Aras Pranckevicius requested review from Harley Acheson 2024-05-04 07:09:33 +02:00

Aras Pranckevicius changed title from ~~WIP: UI: Automatic text contrast for overlays via outline~~ to UI: Automatic text contrast for overlays via outline

2024-05-04 07:09:52 +02:00

Harley Acheson commented

2024-05-04 19:51:48 +02:00

This is beautiful. It gives the result I was always trying to get but couldn't do so in a performant way.

The first thing I notice is that we have a bug in our current themes. Although our current shadow size only supports 0, 3, & 5 (with 0 being off) our default themes (dark and light) both set a shadow size of "1" for Widget (Preferences / Themes / Text Style). This obviously has no affect now but this PR causes shadowing for all widget text. We can deal with this in theme versioning.

In Preferences / Themes / Text style we'd definitely have to change shadow size to an enum rather than a numerical range, especially with "1" being so much stronger looking than the others.

I could be wrong but I think this PR alters the difference between 3 and 5 shadows. The 3 shadow looks slightly lighter and the 5 looks a fair bit lighter. These two have always been a bit too close, but they seems closer now. Obviously the absolute amount of shadow does not have to stay the same. It is just that the progression between these two has always been a bit small and seems smaller.

In the following capture, the top two are from 4.1, the bottom two with this patch. It is subtle but the left edges of the left "p" is lighter. And the left side of the right "p" seems a little lighter with a little quicker falloff. Again, the amount these things is less important, but the overall effect seems to result in little difference between the bottom 3px and 5px. Or maybe this is all too subtle and should be considered a separate issue. Or we could even get rid of the 3px and simplify things down to just None, Outline, Shadow.

This is beautiful. It gives the result I was always trying to get but couldn't do so in a performant way. The first thing I notice is that we have a bug in our current themes. Although our current shadow size only supports 0, 3, & 5 (with 0 being off) our default themes (dark and light) both set a shadow size of "1" for Widget (Preferences / Themes / Text Style). This obviously has no affect now but this PR causes shadowing for all widget text. We can deal with this in theme versioning. In Preferences / Themes / Text style we'd definitely have to change shadow size to an enum rather than a numerical range, especially with "1" being so much stronger looking than the others. I could be wrong but I think this PR alters the difference between 3 and 5 shadows. The 3 shadow looks _slightly_ lighter and the 5 looks a fair bit lighter. These two have always been a bit too close, but they seems closer now. Obviously the absolute amount of shadow does not have to stay the same. It is just that the progression between these two has always been a bit small and seems smaller. In the following capture, the top two are from 4.1, the bottom two with this patch. It is subtle but the left edges of the left "p" is lighter. And the left side of the right "p" seems a little lighter with a little quicker falloff. Again, the amount these things is less important, but the overall effect seems to result in little difference between the bottom 3px and 5px. Or maybe this is all too subtle and should be considered a separate issue. Or we could even get rid of the 3px and simplify things down to just None, Outline, Shadow. ![image](/attachments/a306b74c-ae75-4a0b-80cd-13b66783e4d2)

image.png

21 KiB

Aras Pranckevicius commented

2024-05-04 21:56:28 +02:00

The first thing I notice is that we have a bug in our current themes. Although our current shadow size only supports 0, 3, & 5 (with 0 being off) our default themes (dark and light) both set a shadow size of "1" for Widget (Preferences / Themes / Text Style). This obviously has no affect now but this PR causes shadowing for all widget text. We can deal with this in theme versioning.

Wouldn't it be easier to change "outline" to use value of, say, 6 instead?

I could be wrong but I think this PR alters the difference between 3 and 5 shadows

I'll check it, definitely not intentional on my side.

> The first thing I notice is that we have a bug in our current themes. Although our current shadow size only supports 0, 3, & 5 (with 0 being off) our default themes (dark and light) both set a shadow size of "1" for Widget (Preferences / Themes / Text Style). This obviously has no affect now but this PR causes shadowing for all widget text. We can deal with this in theme versioning. Wouldn't it be easier to change "outline" to use value of, say, 6 instead? > I could be wrong but I think this PR alters the difference between 3 and 5 shadows I'll check it, definitely not intentional on my side.

Harley Acheson commented

2024-05-04 23:47:52 +02:00

Wouldn't it be easier to change "outline" to use value of, say, 6 instead?

Well, that's true and then would progress a bit better in strength.

I'll check it, definitely not intentional on my side.

Also possible that 3 & 5 have always been too similar, and the difference I am seeing could be just a difference in sub-pixel positioning between the two tests.

> Wouldn't it be easier to change "outline" to use value of, say, 6 instead? Well, that's true and then would progress a bit better in strength. > I'll check it, definitely not intentional on my side. Also possible that 3 & 5 have always been too similar, and the difference I am seeing could be just a difference in sub-pixel positioning between the two tests.

Harley Acheson commented

2024-05-05 00:12:25 +02:00

There is one thing I have found that is a little worse with this PR. With default settings, File / New / 2D Animation. On Frame one you should see the object name as "(1) Stroke" (drawn in draw_selected_name). That pukey green looks better with a dark background than light.

We are first creating a text outline color based on the background color and then choosing a contrasting text color. This works great except when we then change the text color to something arbitrary like this and should be using an outline that contrasts with that instead.

Maybe the generic solution is a function that returns a contrasting text color from a background color? Then for most usages we can use the color for TH_BACK, and for 3DView we can use the result of ED_view3d_background_color_get. Once we have the text color we pass that to a function that returns a contrasting shadow color? That way we'd have a different background for this pukey green than for black or white.

There is one thing I have found that is a little worse with this PR. With default settings, File / New / 2D Animation. On Frame one you should see the object name as "(1) Stroke" (drawn in draw_selected_name). That pukey green looks better with a dark background than light. We are first creating a text outline color based on the background color and then choosing a contrasting text color. This works great except when we then change the text color to something arbitrary like this and should be using an outline that contrasts with that instead. Maybe the generic solution is a function that returns a contrasting text color from a background color? Then for most usages we can use the color for TH_BACK, and for 3DView we can use the result of ED_view3d_background_color_get. Once we have the text color we pass that to a function that returns a contrasting shadow color? That way we'd have a different background for this pukey green than for black or white.

Aras Pranckevicius added 3 commits 2024-05-05 10:32:44 +02:00

Merge branch 'main' into text-shadow-contrast 85f13da0b7

BLF: change outline to be enum value 6 5f6d1153e5

Some existing theme options have font style shadow level with value 1,
which is not strictly legal, but happened to behave the same as value
3 would have before.

BLF: fix missign shadow color assignment in DRW text caches (used by geonodes view node), factor out ED_view3d_text_colors_get function 833d21474d

Aras Pranckevicius added 1 commit 2024-05-05 19:12:26 +02:00

Tweak overlay text/shadow colors logic 38f0d04dfd

Aras Pranckevicius commented

2024-05-05 19:16:40 +02:00

Maybe the generic solution is a function that returns a contrasting text color from a background color?

Hmm not sure. What I tried right now (pushed on this branch), is logic like this instead:

Default text color is white, outline color is black.
Unless the 3d view background color is "very dark", in that case default text color is black, outline is white.

This does look "pretty okay" to me, including the case of "File / New / 2D Animation":

(I also pushed change of outline enum to be 6, so that various widgets keep on using the same shadow as before)

> Maybe the generic solution is a function that returns a contrasting text color from a background color? Hmm not sure. What I tried right now (pushed on this branch), is logic like this instead: - Default text color is white, outline color is black. - Unless the 3d view background color is "very dark", in that case default text color is black, outline is white. This does look "pretty okay" to me, including the case of "File / New / 2D Animation": ![2danim-branch-tweaks.png](/attachments/7c43dd0c-7701-448a-90a3-744f57e4f810) (I also pushed change of outline enum to be 6, so that various widgets keep on using the same shadow as before)

2danim-branch-tweaks.png

62 KiB

👎 1

Harley Acheson referenced this pull request

2024-05-05 23:18:32 +02:00

WIP: UI: Automatic Text Contrast for Overlays #121239

Harley Acheson commented

2024-05-06 18:02:08 +02:00

This does look "pretty okay" to me, including the case of "File / New / 2D Animation":

Yes, looks pretty nice to me too.

Wouldn't FontShadowType ::simple be better described as FontShadowType ::none ?

> This does look "pretty okay" to me, including the case of "File / New / 2D Animation": Yes, looks pretty nice to me too. Wouldn't `FontShadowType ::simple` be better described as `FontShadowType ::none` ?

Harley Acheson commented

2024-05-06 18:24:18 +02:00

Otherwise I would take this pretty much as it is. The big (wonderful) changes here are

Improvements to the shader to include a solid outline
The use of ED_view3d_text_colors_get to set the overlay color and shadow based on the view3d background color.

Fairly certain that BLF_draw_default_shadowed can be removed afterward, but attempting that here would just make this more complicated.

We'd probably also want to increase the range and change description in rna_def_userdef_theme_ui_font_style, shadow.

Otherwise I would take this pretty much as it is. The big (wonderful) changes here are * Improvements to the shader to include a solid outline * The use of ED_view3d_text_colors_get to set the overlay color and shadow based on the view3d background color. Fairly certain that `BLF_draw_default_shadowed` can be removed afterward, but attempting that here would just make this more complicated. We'd probably also want to increase the range and change description in `rna_def_userdef_theme_ui_font_style`, shadow.

Aras Pranckevicius commented

2024-05-07 12:37:01 +02:00

Wouldn't FontShadowType ::simple be better described as FontShadowType ::none ?

Well, that option is really like a "simple or none". You can enable shadow, set "simple" and then it draws the drop shadow, but without any blurring or outline processing. So as long as shadow bit is enabled (elsewhere via enablement flags), the actual shadow is more like "simple".

> Wouldn't `FontShadowType ::simple` be better described as `FontShadowType ::none` ? Well, that option is really like a "simple or none". You can enable shadow, set "simple" and then it draws the drop shadow, but without any blurring or outline processing. So as long as shadow bit is enabled (elsewhere via enablement flags), the actual shadow is more like "simple".

Harley Acheson commented

2024-05-08 20:39:42 +02:00

Well, that option is really like a "simple or none". You can enable shadow, set "simple" and then it draws the drop shadow, but without any blurring or outline processing. So as long as shadow bit is enabled (elsewhere via enablement flags), the actual shadow is more like "simple"

Oh, I see. Not trying to be pedantic because you are right. It is just that our existing code seems to (wrongly) assume otherwise.

We normally draw in the UI with a specific uiFontStyle. You see these as separate items in Preferences / Themes / Text Style. uiFontStyle has a (short) "shadow" member with comment of "Value is amount of pixels blur."

When we then print text, in UI_fontstyle_draw_ex, we have a if (fs->shadow) that will then set the BLF_SHADOW flag and setup the shadow color and offset. So it is treating 0 as off.

I guess this only means we have a "simple" shadow choice that we've never been able to use. Is it potentially useful for anything? Would this be a good time to get rid of it? Does it make the shader any simpler?

> Well, that option is really like a "simple or none". You can enable shadow, set "simple" and then it draws the drop shadow, but without any blurring or outline processing. So as long as shadow bit is enabled (elsewhere via enablement flags), the actual shadow is more like "simple" Oh, I see. Not trying to be pedantic because you are right. It is just that our existing code seems to (wrongly) assume otherwise. We normally draw in the UI with a specific uiFontStyle. You see these as separate items in Preferences / Themes / Text Style. uiFontStyle has a (short) "shadow" member with comment of "Value is amount of pixels blur." When we then print text, in `UI_fontstyle_draw_ex`, we have a `if (fs->shadow)` that will then set the BLF_SHADOW flag and setup the shadow color and offset. So it is treating 0 as off. I guess this only means we have a "simple" shadow choice that we've never been able to use. Is it potentially useful for anything? Would this be a good time to get rid of it? Does it make the shader any simpler?

Aras Pranckevicius commented

2024-05-08 21:06:54 +02:00

When we then print text, in UI_fontstyle_draw_ex, we have a if (fs->shadow) that will then set the BLF_SHADOW flag and setup the shadow color and offset. So it is treating 0 as off.
I guess this only means we have a "simple" shadow choice that we've never been able to use. Is it potentially useful for anything? Would this be a good time to get rid of it? Does it make the shader any simpler?

I see. In theory someone could use it, since this whole thing is not only exposed through theme / UI styles, but also has a Python API that people can use however they wish (https://docs.blender.org/api/current/blf.html#blf.shadow).

That said... a quick search across the whole github seems to only ever find people calling it with shadow level values of 3 or 5. Which makes sense; as "no blur" shadow is literally just the same as drawing regular text, just with a different color and possibly at slightly different position.

So yeah I could look into just removing that "shadow with no blur" option. And then rename the enum to None. It would not simplify the shader though, since "no blur" shadow literally just uses the same shader code path as regular text rendering.

> When we then print text, in `UI_fontstyle_draw_ex`, we have a `if (fs->shadow)` that will then set the BLF_SHADOW flag and setup the shadow color and offset. So it is treating 0 as off. > I guess this only means we have a "simple" shadow choice that we've never been able to use. Is it potentially useful for anything? Would this be a good time to get rid of it? Does it make the shader any simpler? I see. _In theory_ someone could use it, since this whole thing is not only exposed through theme / UI styles, but also has a Python API that people can use however they wish (https://docs.blender.org/api/current/blf.html#blf.shadow). That said... a quick search across the _whole github_ seems to only ever find people calling it with shadow level values of 3 or 5. Which makes sense; as "no blur" shadow is literally just the same as drawing regular text, just with a different color and possibly at slightly different position. So yeah I could look into just removing that "shadow with no blur" option. And then rename the enum to `None`. It would not simplify the shader though, since "no blur" shadow literally just uses the same shader code path as regular text rendering.

Harley Acheson commented

2024-05-08 23:09:56 +02:00

I guess a possible alternative is to add a "-1" for off, but just feels like adding more jank to our janky code.

Aras Pranckevicius added 2 commits 2024-05-09 10:21:01 +02:00

Merge branch 'main' into text-shadow-contrast 6776d9aba8

BLF: rename shadow type Simple->None, allow outline in font style ddbaf0bc97

Aras Pranckevicius commented

2024-05-09 10:23:17 +02:00

Alright, pushed a commit that does:

Rename ::Simple to ::None for shadow type in C++ enum,
Allow picking outline (value 6) in theme font style.

I tried making theme font style shadow be an enum, and that almost works, except when loading themes you get a python exception where some themes already store "1" as shadow size, and that then is not within allowed enum values. So scrapped that idea for now, and just extended UI range from 0-5 to 0-6, with tooltip to indicate what means what.

Alright, pushed a commit that does: - Rename `::Simple` to `::None` for shadow type in C++ enum, - Allow picking outline (value 6) in theme font style. I tried making theme font style shadow be an enum, and that almost works, except when loading themes you get a python exception where some themes already store "1" as shadow size, and that then is not within allowed enum values. So scrapped that idea for now, and just extended UI range from 0-5 to 0-6, with tooltip to indicate what means what.

Harley Acheson approved these changes 2024-05-10 20:25:34 +02:00

Harley Acheson left a comment

I think we should take this as it is. And then afterward I'll see about getting rid of BLF_draw_default_shadowed.

Aras Pranckevicius merged commit 3582e52f9c into main

2024-05-10 21:06:59 +02:00

Aras Pranckevicius referenced this issue from a commit

2024-05-10 21:06:59 +02:00

UI: Improved overlay text contrast with new outline text decoration

Aras Pranckevicius deleted branch text-shadow-contrast

2024-05-10 21:07:02 +02:00

Harley Acheson referenced this pull request

2024-05-10 23:32:48 +02:00