Fix #126908: 3D view background color affects attribute text outline color #127071

Aras Pranckevicius · 2024-09-02T13:28:36+02:00

2024-09-02 13:28:36 +02:00

The change was accidentally done in #121383 which primarily concerned
itself with overlay text colors, but started to use TH_BACK theme color
for the draw manager text (e.g. for geometry nodes value visualization)
outline color. Change behavior to use black or white outline color, based
on lightness of text color.

The change was accidentally done in #121383 which primarily concerned itself with overlay text colors, but started to use TH_BACK theme color for the draw manager text (e.g. for geometry nodes value visualization) outline color. Change behavior to use black or white outline color, based on lightness of text color.

Aras Pranckevicius added 1 commit 2024-09-02 13:28:46 +02:00

Fix #126908 : 3D view background color affects attribute text outline color 5c64d14c2a

The change was accidentally done in #121383 which primarily concerned
itself with overlay text colors, but started to use TH_BACK theme color
for the draw manager text (e.g. for geometry nodes value visualization)
outline color. Restore behavior to use fixed "almost black" outline
color.

Aras Pranckevicius requested review from Harley Acheson 2024-09-02 13:28:58 +02:00

Harley Acheson commented

2024-09-02 18:59:50 +02:00

This definitely helps in this case, where we have white text on a white background.

But if the text highlight color is then changed to black then you get a black outline around black text. What do you think of leaving this change for only when the color is light, something like this?

diff --git a/source/blender/draw/intern/draw_manager_text.cc b/source/blender/draw/intern/draw_manager_text.cc
index f0fb6538cc0..0b566ee3c07 100644
--- a/source/blender/draw/intern/draw_manager_text.cc
+++ b/source/blender/draw/intern/draw_manager_text.cc
@@ -143,13 +143,21 @@ static void drw_text_cache_draw_ex(DRWTextStore *dt, ARegion *region)
   const int font_id = BLF_set_default();
 
   float shadow_color[4] = {0, 0, 0, 0.8f};
-  UI_GetThemeColor3fv(TH_BACK, shadow_color);
+  float bg_color[4];
+  UI_GetThemeColor3fv(TH_BACK, bg_color);
 
   BLI_memiter_iter_init(dt->cache_strings, &it);
   while ((vos = static_cast<ViewCachedString *>(BLI_memiter_iter_step(&it)))) {
     if (vos->sco[0] != IS_CLIPPED) {
       if (col_pack_prev != vos->col.pack) {
         BLF_color4ubv(font_id, vos->col.ub);
+        const char lightness = rgb_to_grayscale_byte(vos->col.ub);
+        if (lightness > 153) {
+          copy_v3_fl(shadow_color, 0.0f);
+        }
+        else {
+          copy_v3_v3(shadow_color, bg_color);
+        }
         col_pack_prev = vos->col.pack;
       }

Not sure where the cut off would be to be honest. The above changes it like this:

This definitely helps in this case, where we have white text on a white background. But if the text highlight color is then changed to black then you get a black outline around black text. What do you think of leaving this change for only when the color is light, something like this? ``` diff --git a/source/blender/draw/intern/draw_manager_text.cc b/source/blender/draw/intern/draw_manager_text.cc index f0fb6538cc0..0b566ee3c07 100644 --- a/source/blender/draw/intern/draw_manager_text.cc +++ b/source/blender/draw/intern/draw_manager_text.cc @@ -143,13 +143,21 @@ static void drw_text_cache_draw_ex(DRWTextStore *dt, ARegion *region) const int font_id = BLF_set_default(); float shadow_color[4] = {0, 0, 0, 0.8f}; - UI_GetThemeColor3fv(TH_BACK, shadow_color); + float bg_color[4]; + UI_GetThemeColor3fv(TH_BACK, bg_color); BLI_memiter_iter_init(dt->cache_strings, &it); while ((vos = static_cast<ViewCachedString *>(BLI_memiter_iter_step(&it)))) { if (vos->sco[0] != IS_CLIPPED) { if (col_pack_prev != vos->col.pack) { BLF_color4ubv(font_id, vos->col.ub); + const char lightness = rgb_to_grayscale_byte(vos->col.ub); + if (lightness > 153) { + copy_v3_fl(shadow_color, 0.0f); + } + else { + copy_v3_v3(shadow_color, bg_color); + } col_pack_prev = vos->col.pack; } ``` Not sure where the cut off would be to be honest. The above changes it like this: ![AttributeTextColor.gif](/attachments/3ff72eeb-64c1-4987-b216-8a337602db57)

AttributeTextColor.gif

821 KiB

Aras Pranckevicius commented

2024-09-02 20:10:42 +02:00

@Harley should the outline color alternate between "black" (for bright text) and "background" (for dark text), or instead between "black" and "white"? I'm thinking of case of both TH_BACK and the text color being dark enough, that would still not be very legible right?

Harley Acheson commented

2024-09-02 22:30:33 +02:00

@Harley should the outline color alternate between "black" (for bright text) and "background" (for dark text), or instead between "black" and "white"? I'm thinking of case of both TH_BACK and the text color being dark enough, that would still not be very legible right?

Oh yeah, you're right! Black on black really needs that light outline.

> @Harley should the outline color alternate between "black" (for bright text) and "background" (for dark text), or instead between "black" and "white"? I'm thinking of case of both TH_BACK and the text color being dark enough, that would still not be very legible right? Oh yeah, you're right! Black on black really needs that light outline.

👍 1

Aras Pranckevicius added 1 commit 2024-09-03 08:22:08 +02:00

Use white or black outline color in draw manager text

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.