UI: Automatic text contrast for overlays via outline #121383

Merged
Aras Pranckevicius merged 10 commits from aras_p/blender:text-shadow-contrast into main 2024-05-10 21:06:59 +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:
image
image
image

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)
Aras Pranckevicius added 4 commits 2024-05-03 09:49:55 +02:00
9a75b82ec4 WIP: UI: Automatic Text Contrast for Overlays
Automatic text color and shadows to give contrast for text overlays.
8a302367c4 BLF: add "outline" font shadow type, use enum for shadow types
- 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.
07f7b697ed Merge branch 'main' into text-shadow-contrast
# Conflicts:
#	source/blender/blenfont/intern/blf_glyph.cc
#	source/blender/blenfont/intern/blf_internal_types.hh
Member

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).
Author
Member

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
Member

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

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)
Author
Member

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.
Member

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.
Member

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
Aras Pranckevicius added 1 commit 2024-05-05 19:12:26 +02:00
Author
Member

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

(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)
Member

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` ?
Member

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.
Author
Member

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".
Member

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?
Author
Member

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.
Member

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

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
Author
Member

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
Member

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

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 deleted branch text-shadow-contrast 2024-05-10 21:07:02 +02:00
Harley Acheson added this to the User Interface project 2024-05-15 22:40:30 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser Project (Legacy)
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#121383
No description provided.