blender
/
blender
136
406
Fork 590
Code Issues 6.2k Pull Requests 764 Projects 19 Wiki Activity

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

Merged
Leon Schittek merged 5 commits from lone_noel/blender:fix-95105-unclamp-frame-node-label into main 2023-03-10 18:24:33 +01:00
Conversation 1 Commits 5 Files Changed 2 +21 -6
2 changed files with 21 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 2dd9647365 - Show all commits

17
source/blender/editors/space_node/node_draw.cc
View File

@ -2688,13 +2688,21 @@ static void count_multi_input_socket_links(bNodeTree &ntree, SpaceNode &snode)
}
}
#define NODE_FRAME_MARGIN (1.5f * U.widget_unit)
/* XXX Does a bounding box update by iterating over all children.
* Not ideal to do this in every draw call, but doing as transform callback doesn't work,
* since the child node totr rects are not updated properly at that point. */
static void frame_node_prepare_for_draw(bNode &node, Span<bNode *> nodes)
{
const float margin = 1.5f * U.widget_unit;
NodeFrame *data = (NodeFrame *)node.storage;
const float font_size = data->label_size;
const float margin = NODE_FRAME_MARGIN;
lone_noel marked this conversation as resolved
Iliya Katushenock commented 2023-02-26 21:26:54 +01:00
Review
Copy Link

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
Review
Copy Link

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! :)
/* 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);
/* Initialize rect from current frame size. */
rctf rect;
@ -2715,7 +2723,7 @@ static void frame_node_prepare_for_draw(bNode &node, Span<bNode *> nodes)
noderect.xmin -= margin;
noderect.xmax += margin;
noderect.ymin -= margin;
noderect.ymax += margin;
noderect.ymax += margin_top;
/* First child initializes frame. */
if (bbinit) {
@ -2813,8 +2821,7 @@ static void frame_node_draw_label(TreeDrawContext &tree_draw_ctx,
BLF_enable(fontid, BLF_ASPECT);
BLF_aspect(fontid, aspect, aspect, 1.0f);
/* Clamp. Otherwise it can suck up a LOT of memory. */
BLF_size(fontid, MIN2(24.0f, font_size) * U.dpi_fac);
BLF_size(fontid, font_size * U.dpi_fac);
/* Title color. */
int color_id = node_get_colorid(tree_draw_ctx, node);
@ -2831,7 +2838,7 @@ static void frame_node_draw_label(TreeDrawContext &tree_draw_ctx,
const rctf &rct = node.runtime->totr;
/* XXX a bit hacky, should use separate align values for x and y. */
float x = BLI_rctf_cent_x(&rct) - (0.5f * width);
float y = rct.ymax - label_height;
float y = rct.ymax - label_height - (0.5f * NODE_FRAME_MARGIN);
/* label */
const bool has_label = node.label[0] != '\0';

10
source/blender/makesrna/intern/rna_nodetree.c
View File

@ -11,6 +11,8 @@
#include "BLI_math.h"
#include "BLI_utildefines.h"
#include "BLF_api.h"
#include "BLT_translation.h"
#include "DNA_curves_types.h"
@ -4164,6 +4166,12 @@ static bNodeSocket *rna_NodeOutputFile_slots_new(
return sock;
}
static void rna_FrameNode_label_size_update(Main *bmain, Scene *scene, PointerRNA *ptr)
{
BLF_cache_clear();
rna_Node_update(bmain, scene, ptr);
}
static void rna_ShaderNodeTexIES_mode_set(PointerRNA *ptr, int value)
{
bNode *node = (bNode *)ptr->data;
@ -4816,7 +4824,7 @@ static void def_frame(StructRNA *srna)
RNA_def_property_int_sdna(prop, NULL, "label_size");
RNA_def_property_range(prop, 8, 64);
RNA_def_property_ui_text(prop, "Label Font Size", "Font size to use for displaying the label");
RNA_def_property_update(prop, NC_NODE | ND_DISPLAY, NULL);
RNA_def_property_update(prop, NC_NODE | ND_DISPLAY, "rna_FrameNode_label_size_update");
}
static void def_clamp(StructRNA *srna)