Fix #95105: Unclamp draw size of the frame node's label #104555

Leon Schittek · 2023-02-10T08:06:20+01:00

Leon Schittek commented

2023-02-10 08:06:20 +01:00

Fix issues with the display of frame node labels, that made them
harder to read for people with low vision or on high dpi displays:

Unclamp the size of text that is drawn in the frame node
so all sizes can be displayed consistently at all zoom levels
and independet of the UI scale.
Account for the label when calculating the frame node's
bounds, so child nodes don't obscure the label.

This patch was originally submitted here:
https://archive.blender.org/developer/D13954

There were some more general ideas about how to change the design
of the frame's label that are worth exploring, but I think the
patch is a good improvement to the behavior of the current design
and worth including as a fix.

Comparison
Notice how (a) the labels of different sizes are now displayed the same
relative to the other elements in the node tree in all zoom levels and
(b) the child nodes don't overlap the labels anymore.

	main	patch
1080p
retina

To make it fit with the label, the margin for the text drawn in
the frame node has also been increased.

main	patch

Fix issues with the display of frame node labels, that made them harder to read for people with low vision or on high dpi displays: * Unclamp the size of text that is drawn in the frame node so all sizes can be displayed consistently at all zoom levels and independet of the UI scale. * Account for the label when calculating the frame node's bounds, so child nodes don't obscure the label. --- This patch was originally submitted here: https://archive.blender.org/developer/D13954 There were some more general ideas about how to change the design of the frame's label that are worth exploring, but I think the patch is a good improvement to the behavior of the current design and worth including as a fix. **Comparison** Notice how (a) the labels of different sizes are now displayed the same relative to the other elements in the node tree in all zoom levels and (b) the child nodes don't overlap the labels anymore. ||main|patch| |---|---|---| |**1080p**|![](https://projects.blender.org/attachments/99fdaf35-b9b0-46ee-ba7b-60e901532ae4)|![](https://projects.blender.org/attachments/52553d75-c76c-4950-ac72-687a66112920)| |**retina**|![](https://projects.blender.org/attachments/9093d325-d486-4147-9872-b0bfd9d9a2bd)|![](https://projects.blender.org/attachments/4c82c6ff-af41-4149-8cbf-936acae88c55)| To make it fit with the label, the margin for the text drawn in the frame node has also been increased. |main|patch| |---|---| |![](https://projects.blender.org/attachments/2d7d46b8-b4b1-4183-bc21-c30c74c38b7e)|![](https://projects.blender.org/attachments/14fa580c-def5-489f-84c0-e1140cfe3267)|

master_frame-labels_1080p.png

124 KiB

patch_frame-labels_1080p.png

153 KiB

patch_frame-labels_retina.png

243 KiB

master_frame-labels_retina.png

213 KiB

frame-node-label_test.blend

825 KiB

master_frame-text.png

508 KiB

patch_frame-text.png

468 KiB

👍 2

Leon Schittek added the

Module

Nodes & Physics

label 2023-02-10 08:19:52 +01:00

Brecht Van Lommel added this to the Nodes & Physics project 2023-02-13 09:11:57 +01:00

Iliya Katushenock reviewed 2023-02-26 21:23:33 +01:00

source/blender/editors/space_node/node_draw.cc

						
				@ -2684,13 +2684,21 @@ static void count_multi_input_socket_links(bNodeTree &ntree, SpaceNode &snode)

				  }

				}

				#define NODE_FRAME_MARGIN (1.5f * U.widget_unit)

Iliya Katushenock commented

2023-02-26 21:23:33 +01:00

Are it make sense to move it on top file part?

Leon Schittek commented

2023-03-03 00:29:28 +01:00

I don't really have a preference, but some of the other #define's (e.g. NODE_HEADER_ICON_SIZE) are also declared right before the section they are used in, so I think it makes sense to stay consistent with that for now.

I don't really have a preference, but some of the other #define's (e.g. `NODE_HEADER_ICON_SIZE`) are also declared right before the section they are used in, so I think it makes sense to stay consistent with that for now.

👍 1

lone_noel marked this conversation as resolved

Iliya Katushenock reviewed 2023-02-26 21:26:54 +01:00

source/blender/editors/space_node/node_draw.cc

						
				@ -2694,0 +2698,4 @@

				  /* This is a reasonable heuristic for the margin at the top of the frame to avoid the

				   * node label clipping under the frame's child nodes. */

				  const float has_label = node.label[0] != '\0';

				  const float margin_top = margin + (has_label ? (margin * font_size / 24.0f) : 0.0f);

Iliya Katushenock commented

2023-02-26 21:26:54 +01:00

Seems to like magic numbers, what do you think, should it be more clear if add some comments about why 24.0f?

Seems to like magic numbers, what do you think, should it be more clear if add some comments about why `24.0f`?

Leon Schittek commented

2023-03-03 00:38:10 +01:00

Yeah, looking back at this this doesn't really make sense.

The expression margin * font_size / 24.0f reduces to 1.25f * U.dpi_size * font_size. The 1.25f adds an additional 25% to account for the descender of the font. This doesn't work out for every font, but it seems to be a good enough guess that avoids duplicating the logic for the font size from frame_node_draw_label.

I'll clean this up. Thanks for the feedback! :)

Yeah, looking back at this this doesn't really make sense. The expression `margin * font_size / 24.0f` reduces to `1.25f * U.dpi_size * font_size`. The `1.25f` adds an additional 25% to account for the descender of the font. This doesn't work out for every font, but it seems to be a good enough guess that avoids duplicating the logic for the font size from `frame_node_draw_label`. I'll clean this up. Thanks for the feedback! :)

lone_noel marked this conversation as resolved

Leon Schittek force-pushed fix-95105-unclamp-frame-node-label from 058ef3f38d to d7222bac41

2023-03-04 16:28:19 +01:00

Compare

Brecht Van Lommel approved these changes 2023-03-10 13:07:15 +01:00

Leon Schittek added 1 commit 2023-03-10 17:44:14 +01:00