UI: Color Icon Theme Internal Parts #125146

Harley Acheson · 2024-07-20T22:54:42+02:00

Harley Acheson commented

2024-07-20 22:54:42 +02:00

Allow color SVG icons to have their fill colors modified by theme
colors using a callback. This is done just before rasterization so
only happens once per requested size, before these are cached for
reuse.

As an example this PR alters "record_on.svg" slightly so that, only if it is defined as DEF_ICON_COLOR, it will display its inner circle in red, while the outer ring will show in the current text color and so changing with theme changes:

While in dark mode it would look like this:

While in light mode you notice that the outer ring color is different:

This PR leaves "ICON_RECORD_ON" defined as just DEF_ICON and therefore it looks and behaves as it does currently.

Allow color SVG icons to have their fill colors modified by theme colors using a callback. This is done just before rasterization so only happens once per requested size, before these are cached for reuse. --- As an example this PR alters "record_on.svg" slightly so that, **only if it is defined as DEF_ICON_COLOR**, it will display its inner circle in red, while the outer ring will show in the current text color and so changing with theme changes: While in dark mode it would look like this: ![image](/attachments/29797304-6001-43a4-ba28-ae1b6968518f) While in light mode you notice that the outer ring color is different: ![image](/attachments/47b7dabd-c5f6-4f28-bf0d-35244b069f59) This PR leaves "ICON_RECORD_ON" defined as just DEF_ICON and therefore it looks and behaves as it does currently.

LightRecord.gif

7.6 KiB

DarkRecord.gif

7.7 KiB

👍 4 ❤️ 7

Harley Acheson added this to the User Interface project 2024-07-20 23:03:33 +02:00

Harley Acheson requested review from Hans Goudey 2024-07-20 23:03:43 +02:00

Harley Acheson referenced this pull request

2024-07-24 18:48:38 +02:00

UI: Use SVG Icons for Tools #125378

Harley Acheson referenced this pull request

2024-07-31 22:32:48 +02:00

WIP: Proof of Concept for Color UI Icons #123950

Harley Acheson force-pushed IconThemeColor from c96caf2e4a to 4ed805e53a

2024-08-07 23:47:47 +02:00

Compare

Harley Acheson added 2 commits 2024-08-08 02:16:25 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor 52e65dbb5c

Working better with Tool Icon colors f97ec1d8d9

Hans Goudey reviewed 2024-08-08 21:51:08 +02:00

source/blender/blenfont/intern/blf_internal.hh Outdated

						
				@ -109,2 +111,3 @@

				                       float outline_alpha = 1.0f,

				                       bool multicolor = false);

				                       bool multicolor = false,

				                       std::function<void(std::string &)> edit_source_cb = nullptr);

Hans Goudey commented

2024-08-08 21:38:53 +02:00

FunctionRef would be a better fit here. std::function is only used when it needs to own data (i.e. the function is passed around and saved for later).

`FunctionRef` would be a better fit here. `std::function` is only used when it needs to own data (i.e. the function is passed around and saved for later).

Harley marked this conversation as resolved

Harley Acheson added 2 commits 2024-08-08 23:44:16 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor 0ba3ce6673

Replace std::function with FunctionRef as per review by Hans cbaa439921

Hans Goudey approved these changes 2024-08-09 17:02:46 +02:00

source/blender/editors/interface/interface_icons.cc Outdated

						
				@ -1296,2 +1298,4 @@

				}

				static void svg_replace_color_attributes(std::string &svg,

				                                         const std::string name,

Hans Goudey commented

2024-08-09 17:01:44 +02:00

Pass std::string by const reference rather than by value.

Generally I'm a bit concerned by the performance of this function when it starts acting on thousands of icons. There are a lot of incremental updates to the text, and then it's called once for every theme color.

I don't want to get in the way or request premature optimizations, but I'd just suggest keeping an eye on this as more SVG icons are processed with this function. It shouldn't have an impact on startup time for example.

Pass `std::string` by const reference rather than by value. Generally I'm a bit concerned by the performance of this function when it starts acting on thousands of icons. There are a lot of incremental updates to the text, and then it's called once for every theme color. I don't want to get in the way or request premature optimizations, but I'd just suggest keeping an eye on this as more SVG icons are processed with this function. It shouldn't have an impact on startup time for example.

Harley Acheson commented

2024-08-09 17:55:43 +02:00

Agreed. I'll add a "Todo" comment to icon_source_edit_cb indicating that this text processing is suboptimal with all the inserts and has lots of room for optimization.

Agreed. I'll add a "Todo" comment to `icon_source_edit_cb` indicating that this text processing is suboptimal with all the inserts and has lots of room for optimization.

Harley Acheson commented

2024-08-09 21:59:04 +02:00

Actually re-wrote that section so the processing is done by scanning the string by group, only for those that start with "blender."

Harley marked this conversation as resolved

Harley Acheson added 2 commits 2024-08-09 21:57:45 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor e9f2fb10fe

Process SVG source string by group id, if starting with "blender." 8b613ed72f

Hans Goudey reviewed 2024-08-09 22:14:34 +02:00

source/blender/editors/interface/interface_icons.cc Outdated

						
				@ -1298,0 +1336,4 @@

				  };

				  for (const ColorItem &item : items) {

				    if (name.compare(item.name) != 0) {

Hans Goudey commented

2024-08-09 22:14:16 +02:00

Why not just name != item.name?

Why not just `name != item.name`?

Harley marked this conversation as resolved

source/blender/editors/interface/interface_icons.cc Outdated

						
				@ -1298,0 +1365,4 @@

				      if (att_start == std::string::npos || att_start > end) {

				        break;

				      }

				      const size_t att_end = svg.find("\"", att_start + 6);

Hans Goudey commented

2024-08-09 22:13:22 +02:00

These numbers 6, 20, 14, 16, should come from somewhere. I imagine they're the length of some string? Probably better to compute it at compile time then. For example with StringRef::size().

These numbers 6, 20, 14, 16, should come from somewhere. I imagine they're the length of some string? Probably better to compute it at compile time then. For example with `StringRef::size()`.

Harley marked this conversation as resolved

Hans Goudey requested review from Hans Goudey 2024-08-09 22:14:46 +02:00

Harley Acheson added 2 commits 2024-08-10 00:02:08 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor 80d3846ee1

Use constexpr for offset values as per review by Hans 3985dba8d0

Harley Acheson added 2 commits 2024-08-10 00:04:58 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor c784733306

Just use != for the std::string to c_str comparison as per review by Hans 48b92c1241

Harley Acheson added 1 commit 2024-08-10 00:36:33 +02:00

Let's add the current custom icon theme colors (scene, folder, fund, etc) 2195984e68

Harley Acheson added 2 commits 2024-08-11 19:02:18 +02:00

Merge branch 'main' of projects.blender.org:blender/blender into IconThemeColor d5d9969c30