blender
/
blender
134
399
Fork 581
Code Issues 6.3k Pull Requests 757 Projects 19 Wiki Activity

I18n: add per-socket translation contexts for nodes #105195

Merged
Hans Goudey merged 3 commits from pioverfour/blender:dp_per_node_socket_translation_context into main 2023-03-06 14:24:49 +01:00
Conversation 2 Commits 3 Files Changed 4 +42 -3
4 changed files with 42 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
source/blender/compositor/realtime_compositor/CMakeLists.txt
View File

@ -6,6 +6,7 @@ set(INC
cached_resources
../../blenkernel
../../blenlib
../../blentranslation
../../gpu
../../imbuf
../../makesdna

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

@ -181,6 +181,23 @@ void ED_node_tag_update_id(ID *id)
namespace blender::ed::space_node {
static const char *node_socket_get_translation_context(const bNodeSocket &socket)
{
/* The node is not explicitly defined. */
if (socket.runtime->declaration == nullptr) {
return nullptr;
}
Damien Picard commented 2023-02-25 01:59:32 +01:00
Review
Copy Link

This is needed to avoid a crash when certain nodes are added (for instance, Render Layers in the compositing nodes).

GCC gives a warning:
the compiler can assume that the address of ‘socket_decl’ will never be NULL [-Waddress].

I suppose it means this should not happen because a null pointer would mean a bug occurs before reaching this point. But I don’t know how to fix this better since it appears not all nodes have a declaration.

This is needed to avoid a crash when certain nodes are added (for instance, Render Layers in the compositing nodes). GCC gives a warning: `the compiler can assume that the address of ‘socket_decl’ will never be NULL [-Waddress]`. I suppose it means this should not happen because a null pointer would mean a bug occurs before reaching this point. But I don’t know how to fix this better since it appears not all nodes have a declaration.
Bastien Montagne commented 2023-02-27 14:49:46 +01:00
Review
Copy Link

You cannot use a reference if there is a chance that the pointer will be NULL. So if socket.runtime->declaration can be nullptr, you need to use a pointer for socket_decl - or do the check before assigning it to your reference.

You cannot use a reference if there is a chance that the pointer will be NULL. So if `socket.runtime->declaration` can be `nullptr`, you need to use a pointer for `socket_decl` - or do the check before assigning it to your reference.
Iliya Katushenock commented 2023-02-27 14:55:51 +01:00
Review
Copy Link

At first check socket.runtime->declaration != nullptr, and after that dereference pointer

At first check `socket.runtime->declaration != nullptr`, and after that dereference pointer
Damien Picard commented 2023-02-27 16:21:22 +01:00
Review
Copy Link

Thanks for the help! I hope I fixed this correctly.

Thanks for the help! I hope I fixed this correctly.
blender::StringRefNull translation_context = socket.runtime->declaration->translation_context;
/* Default context. */
if (translation_context.is_empty()) {
return nullptr;
}
return translation_context.data();
pioverfour marked this conversation as resolved
Hans Goudey commented 2023-02-28 05:33:07 +01:00
Review
Copy Link

Why BLI_strdup? Could you just use StringRefNull and return that directly? It looks like the result isn't being freed, and this negates the benefit of any small string optimization in std::string

Why `BLI_strdup`? Could you just use `StringRefNull` and return that directly? It looks like the result isn't being freed, and this negates the benefit of any small string optimization in `std::string`
}
static void node_socket_add_tooltip_in_node_editor(const bNodeTree &ntree,
pioverfour marked this conversation as resolved
Bastien Montagne commented 2023-02-27 15:16:39 +01:00
Review
Copy Link

Not sure why you are using an intermediate std::stringstream here? Can't you just do return BLI_strdup(socket_decl.translation_context.c_str()); ?

Not sure why you are using an intermediate `std::stringstream` here? Can't you just do `return BLI_strdup(socket_decl.translation_context.c_str());` ?
Jacques Lucke commented 2023-02-27 15:54:43 +01:00
Review
Copy Link

That sounds reasonable.
I guess the reason for why the std::stringstream was used is that you assigned it to a StringRef which does not have c_str method, because it might not be null-terminated. Using const StringRefNull or what @mont29 would be better though.

That sounds reasonable. I guess the reason for why the `std::stringstream` was used is that you assigned it to a `StringRef` which does not have `c_str` method, because it might not be null-terminated. Using `const StringRefNull` or what @mont29 would be better though.
Damien Picard commented 2023-02-27 16:23:30 +01:00
Review
Copy Link

To be quite honest I copied code doing a similar thing from node_socket_get_tooltip(), but I see that it doesn’t really apply here.
I’ll go with @mont29 ’s solution.

To be quite honest I copied code doing a similar thing from `node_socket_get_tooltip()`, but I see that it doesn’t really apply here. I’ll go with @mont29 ’s solution.
const bNodeSocket &sock,
uiLayout &layout);
@ -377,8 +394,14 @@ static void node_update_basis(const bContext &C,
/* Align output buttons to the right. */
uiLayout *row = uiLayoutRow(layout, true);
uiLayoutSetAlignment(row, UI_LAYOUT_ALIGN_RIGHT);
const char *socket_label = nodeSocketLabel(socket);
socket->typeinfo->draw((bContext *)&C, row, &sockptr, &nodeptr, IFACE_(socket_label));
const char *socket_translation_context = node_socket_get_translation_context(*socket);
socket->typeinfo->draw((bContext *)&C,
row,
&sockptr,
&nodeptr,
CTX_IFACE_(socket_translation_context, socket_label));
node_socket_add_tooltip_in_node_editor(ntree, *socket, *row);
@ -511,7 +534,12 @@ static void node_update_basis(const bContext &C,
uiLayout *row = uiLayoutRow(layout, true);
const char *socket_label = nodeSocketLabel(socket);
socket->typeinfo->draw((bContext *)&C, row, &sockptr, &nodeptr, IFACE_(socket_label));
const char *socket_translation_context = node_socket_get_translation_context(*socket);
socket->typeinfo->draw((bContext *)&C,
row,
&sockptr,
&nodeptr,
CTX_IFACE_(socket_translation_context, socket_label));
node_socket_add_tooltip_in_node_editor(ntree, *socket, *row);

9
source/blender/nodes/NOD_node_declaration.hh
View File

@ -8,6 +8,8 @@
#include "BLI_string_ref.hh"
#include "BLI_vector.hh"
#include "BLT_translation.h"
#include "DNA_node_types.h"
struct bNode;
@ -145,6 +147,7 @@ class SocketDeclaration {
std::string name;
std::string identifier;
std::string description;
std::string translation_context;
/** Defined by whether the socket is part of the node's input or
* output socket declaration list. Included here for convenience. */
eNodeSocketInOut in_out;
@ -275,6 +278,12 @@ class SocketDeclarationBuilder : public BaseSocketDeclarationBuilder {
return *(Self *)this;
}
Self &translation_context(std::string value = BLT_I18NCONTEXT_DEFAULT)
{
decl_->translation_context = std::move(value);
return *(Self *)this;
}
Self &no_muted_links(bool value = true)
{
decl_->no_mute_links = value;

3
source/blender/nodes/composite/nodes/node_composite_keyingscreen.cc
View File

@ -33,7 +33,8 @@ namespace blender::nodes::node_composite_keyingscreen_cc {
static void cmp_node_keyingscreen_declare(NodeDeclarationBuilder &b)
{
b.add_output<decl::Color>(N_("Screen"));
b.add_output<decl::Color>(CTX_N_(BLT_I18NCONTEXT_ID_SCREEN, "Screen"))
.translation_context(BLT_I18NCONTEXT_ID_SCREEN);
}
static void node_composit_init_keyingscreen(const bContext *C, PointerRNA *ptr)