Nodes: panel buttons, short names and material properties #112591

Brecht Van Lommel · 2023-09-19T19:05:22+02:00

Brecht Van Lommel commented

2023-09-19 19:05:22 +02:00

This is a set of changes to improve the Principled BSDF panels.

Short socket names

Shortening names based on context is something we do in the user
interface throughout Blender. This is a simple automatic mechanism to do
it for node panels as well, which significantly improves the Principled
BSDF layout.

We want the properties and sockets to be identifiable outside of the
panel context, for example in the animation editor, API docs, or
when exposing a single socket as a group input. So making the actual
name shorter is not an option.

The alternative would be to add manual control over this, perhaps by
giving sockets both a "label" and "short label". But I'm not sure it's
worth doing that.

Buttons inside panels

With a callback to node panels similar to the one for nodes. Used in the
Principled BSDF to place enums in the relevant panels.

Longer term we want to make enums into sockets, but even then there are
still potentially some types of buttons we want to have in panels.

Panels in material properties

The layout for this is not great but consistent with existing
collapse/expand functionality there. There are ideas to replace this
entire material properties drawing longer term, but for 4.0 it's
important to show the panels in some way.

This is a set of changes to improve the Principled BSDF panels. ### Short socket names Shortening names based on context is something we do in the user interface throughout Blender. This is a simple automatic mechanism to do it for node panels as well, which significantly improves the Principled BSDF layout. We want the properties and sockets to be identifiable outside of the panel context, for example in the animation editor, API docs, or when exposing a single socket as a group input. So making the actual name shorter is not an option. The alternative would be to add manual control over this, perhaps by giving sockets both a "label" and "short label". But I'm not sure it's worth doing that. ### Buttons inside panels With a callback to node panels similar to the one for nodes. Used in the Principled BSDF to place enums in the relevant panels. Longer term we want to make enums into sockets, but even then there are still potentially some types of buttons we want to have in panels. ### Panels in material properties The layout for this is not great but consistent with existing collapse/expand functionality there. There are ideas to replace this entire material properties drawing longer term, but for 4.0 it's important to show the panels in some way.

after.png

184 KiB

before.png

220 KiB

👍 3 ❤️ 5 🎉 2

Brecht Van Lommel requested review from Lukas Tönne 2023-09-19 19:05:45 +02:00

Brecht Van Lommel requested review from Hans Goudey 2023-09-19 19:05:45 +02:00

Iliya Katushenock reviewed 2023-09-19 19:10:52 +02:00

source/blender/editors/space_node/node_draw.cc Outdated

						
				@ -401,0 +407,4 @@

				  /* Shorten socket label if it begins with the panel label. */

				  if (translated_panel_label &&

				      STREQLEN(translated_socket_label, translated_panel_label, len_prefix) &&

Iliya Katushenock commented

2023-09-19 19:10:52 +02:00

StringRef::startswith(...), StringRef::trim(...), ...?

`StringRef::startswith(...)`, `StringRef::trim(...)`, ...?

Brecht Van Lommel commented

2023-09-20 18:40:53 +02:00

I don't really see the point, the resulting code is more complicated. If we were using StringRef throughout the UI code then it would make sense, but using it in places like this does not seem useful to me.

I don't really see the point, the resulting code is more complicated. If we were using `StringRef` throughout the UI code then it would make sense, but using it in places like this does not seem useful to me.

mod_moder marked this conversation as resolved

Brecht Van Lommel referenced this pull request

2023-09-19 19:11:11 +02:00

UI: improve vertical spacing in node panels #112593

Brecht Van Lommel referenced this pull request

2023-09-19 19:27:44 +02:00

Principled v2 BSDF #99447

Brecht Van Lommel force-pushed node-panel-shorten-name from ef26ee1b73 to 50e81b9211

2023-09-20 18:46:56 +02:00

Compare

Brecht Van Lommel changed title from ~~UI: shorten node socket labels when they start with the panel name~~ to Nodes: panel buttons, short names and material properties

2023-09-20 18:47:30 +02:00

Brecht Van Lommel force-pushed node-panel-shorten-name from 50e81b9211 to 07279b8c8e

2023-09-20 18:56:31 +02:00

Compare

Brecht Van Lommel referenced this pull request

2023-09-20 18:57:17 +02:00

Add panels to Principled BSDF node #112314

Brecht Van Lommel commented

2023-09-20 19:00:46 +02:00

I added more changes for the Principled BSDF and update the screenshots. I could separate it into separate PRs, but the 3 commits depend on each other so I think it's easier to test this way.

Hans Goudey requested changes 2023-09-20 19:11:15 +02:00

Hans Goudey left a comment

This looks pretty straightforward, I like the choices you've made here. Just have some inline comments.

For sockets like "Specular" in the "Specular" panel, it would they be nice if they had names like "Specular Strength" so they just show as "Strength" inside the panel. What do you think about that?

This looks pretty straightforward, I like the choices you've made here. Just have some inline comments. For sockets like "Specular" in the "Specular" panel, it would they be nice if they had names like "Specular Strength" so they just show as "Strength" inside the panel. What do you think about that?

source/blender/editors/space_node/node_draw.cc Outdated

						
				@ -398,9 +402,28 @@ static bool node_update_basis_buttons(

				  return true;

				}

				const char *node_socket_get_label(bNodeSocket *socket, const char *panel_label)

Hans Goudey commented

2023-09-20 19:04:00 +02:00

bNodeSocket *socket -> const bNodeSocket *socket

`bNodeSocket *socket` -> `const bNodeSocket *socket`

brecht marked this conversation as resolved

source/blender/editors/space_node/node_draw.cc Outdated

						
				@ -401,0 +415,4 @@

				  {

				    return translated_socket_label + len_prefix + 1;

				  }

				  else {

Hans Goudey commented

2023-09-20 19:02:45 +02:00

else after return

brecht marked this conversation as resolved

source/blender/nodes/NOD_node_declaration.hh

						
				@ -569,6 +572,8 @@ class SocketDeclarationBuilder : public BaseSocketDeclarationBuilder {

				using SocketDeclarationPtr = std::unique_ptr<SocketDeclaration>;

				typedef void (*PanelDrawButtonsFunction)(uiLayout *, bContext *, PointerRNA *);

Hans Goudey commented

2023-09-20 19:07:04 +02:00

using PanelDrawButtonsFunction = void (*)(uiLayout *, bContext *, PointerRNA *);

Also: bContext * -> const bContext *

`using PanelDrawButtonsFunction = void (*)(uiLayout *, bContext *, PointerRNA *);` Also: `bContext *` -> `const bContext *`

Brecht Van Lommel commented

2023-09-20 19:26:02 +02:00

I kept the callback the same as the existing draw_buttons one so I could share the node_update_basis_socket implementation. I'd prefer to change both after this PR.

I kept the callback the same as the existing `draw_buttons` one so I could share the `node_update_basis_socket` implementation. I'd prefer to change both after this PR.

👍 1

source/blender/nodes/shader/nodes/node_shader_bsdf_principled.cc Outdated

						
				@ -56,2 +66,3 @@

				  /* Panel for Subsurface scattering settings. */

				  PanelDeclarationBuilder &sss = b.add_panel("Subsurface").default_closed(true);

				  PanelDeclarationBuilder &sss =

				      b.add_panel("Subsurface").default_closed(true).draw_buttons(node_buts_subsurface);

Hans Goudey commented

2023-09-20 19:08:29 +02:00

A lambda with no captures can be passed as a function pointer:

  PanelDeclarationBuilder &sss =
      b.add_panel("Subsurface")
          .default_closed(true)
          .draw_buttons([](uiLayout *layout, bContext * /*C*/, PointerRNA *ptr) {
            uiItemR(layout, ptr, "subsurface_method", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE);
          });

A lambda with no captures can be passed as a function pointer: ``` PanelDeclarationBuilder &sss = b.add_panel("Subsurface") .default_closed(true) .draw_buttons([](uiLayout *layout, bContext * /*C*/, PointerRNA *ptr) { uiItemR(layout, ptr, "subsurface_method", UI_ITEM_R_SPLIT_EMPTY_NAME, "", ICON_NONE); }); ```

brecht marked this conversation as resolved

Brecht Van Lommel commented

2023-09-20 19:26:25 +02:00

For sockets like "Specular" in the "Specular" panel, it would they be nice if they had names like "Specular Strength" so they just show as "Strength" inside the panel. What do you think about that?

We are considering this. The name compatible with Standard Surface would be "Weight" instead of "Strength". But that decision can be made separate from this PR.

> For sockets like "Specular" in the "Specular" panel, it would they be nice if they had names like "Specular Strength" so they just show as "Strength" inside the panel. What do you think about that? We are considering this. The name compatible with Standard Surface would be "Weight" instead of "Strength". But that decision can be made separate from this PR.

👍 1

Brecht Van Lommel added 1 commit 2023-09-20 19:51:41 +02:00

d8a9ed32c7 Address comments

Hans Goudey approved these changes 2023-09-20 23:33:01 +02:00

Brecht Van Lommel referenced this issue from a commit

2023-09-21 18:46:51 +02:00

UI: shorten node socket labels when they start with the panel name

Brecht Van Lommel referenced this issue from a commit

2023-09-21 18:46:51 +02:00

UI: draw node panels in material properties editor

Brecht Van Lommel referenced this issue from a commit

2023-09-21 18:46:51 +02:00