This adds support for attaching gizmos for input values. The goal is to make it easier for users to set input values intuitively in the 3D viewport.
We went through multiple different possible designs until we settled on the one implemented here. We picked it for it's flexibility and ease of use when using geometry node assets. The core principle in the design is that gizmos are attached to existing input values instead of being the input value themselves. This actually fits the existing concept of gizmos in Blender well, but may be a bit unintutitive in a node setup at first. The attachment is done using links in the node editor.
The most basic usage of the node is to link a Value node to the new Linear Gizmo node. This attaches the gizmo to the input value and allows you to change it from the 3D view. The attachment is indicated by the gizmo icon in the sockets which are controlled by a gizmo as well as the back-link (notice the double link) when the gizmo is active.
The core principle makes it straight forward to control the same node setup from the 3D view with gizmos, or by manually changing input values, or by driving the input values procedurally.
If the input value is controlled indirectly by other inputs, it's often possible to automatically propagate the gizmo to the actual input.
Backpropagation does not work for all nodes, although more nodes can be supported over time.
This patch adds the first three gizmo nodes which cover common use cases:
Linear Gizmo: Creates a gizmo that controls a float or integer value using a linear movement of e.g. an arrow in the 3D viewport.
Dial Gizmo: Creates a circular gizmo in the 3D viewport that can be rotated to change the attached angle input.
Transform Gizmo: Creates a simple gizmo for location, rotation and scale.
In the future, more built-in gizmos and potentially the ability for custom gizmos could be added.
All gizmo nodes have a Transform geometry output. Using it is optional but it is recommended when the gizmo is used to control inputs that affect a geometry. When it is used, Blender will automatically transform the gizmos together with the geometry that they control. To achieve this, the output should be merged with the generated geometry using the Join Geometry node. The data contained in Transform output is not visible geometry, but just internal information that helps Blender to give a better user experience when using gizmos.
The gizmo nodes have a multi-input socket. This allows controlling multiple values with the same gizmo.
Only a small set of gizmo shapes is supported initially. It might be extended in the future but one goal is to give the gizmos used by different node group assets a familiar look and feel. A similar constraint exists for colors. Currently, one can choose from a fixed set of colors which can be modified in the theme settings.
The set of visible gizmos is determined by a multiple factors because it's not really feasible to show all possible gizmos at all times. To see any of the geometry nodes gizmos, the "Active Modifier" option has to be enabled in the "Viewport Gizmos" popover. Then all gizmos are drawn for which at least one of the following is true:
The gizmo controls an input of the active modifier of the active object.
The gizmo controls a value in a selected node in an open node editor.
The gizmo controls a pinned value in an open node editor. Pinning works by clicking the gizmo icon next to the value.
Potential future improvements:
More gizmo nodes.
More inverse implementations of nodes.
More fine grained control over which gizmos are shown.
Support propagating gizmos back to attributes.
Support propagating gizmos through drivers.
Add screen space and 2D gizmos to transform gizmo.
Support holding down the alt key to ignore soft limits.
This adds support for attaching gizmos for input values. The goal is to make it easier for users to set input values intuitively in the 3D viewport.
We went through multiple different possible designs until we settled on the one implemented here. We picked it for it's flexibility and ease of use when using geometry node assets. The core principle in the design is that **gizmos are attached to existing input values instead of being the input value themselves**. This actually fits the existing concept of gizmos in Blender well, but may be a bit unintutitive in a node setup at first. The attachment is done using links in the node editor.
The most basic usage of the node is to link a Value node to the new Linear Gizmo node. This attaches the gizmo to the input value and allows you to change it from the 3D view. The attachment is indicated by the gizmo icon in the sockets which are controlled by a gizmo as well as the back-link (notice the double link) when the gizmo is active.
![image](/attachments/2009752d-0bb2-439c-96ec-19507608084f)
The core principle makes it straight forward to control the same node setup from the 3D view with gizmos, or by manually changing input values, or by driving the input values procedurally.
![image](/attachments/3a8474c3-09e4-4184-9605-2bb417065984)
If the input value is controlled indirectly by other inputs, it's often possible to **automatically propagate** the gizmo to the actual input.
![image](/attachments/3c506c48-8413-4bbd-93e9-ebaf767968d9)
Backpropagation does not work for all nodes, although more nodes can be supported over time.
This patch adds the first three gizmo nodes which cover common use cases:
* **Linear Gizmo**: Creates a gizmo that controls a float or integer value using a linear movement of e.g. an arrow in the 3D viewport.
* **Dial Gizmo**: Creates a circular gizmo in the 3D viewport that can be rotated to change the attached angle input.
* **Transform Gizmo**: Creates a simple gizmo for location, rotation and scale.
In the future, more built-in gizmos and potentially the ability for custom gizmos could be added.
All gizmo nodes have a **Transform** geometry output. Using it is optional but it is recommended when the gizmo is used to control inputs that affect a geometry. When it is used, Blender will automatically transform the gizmos together with the geometry that they control. To achieve this, the output should be merged with the generated geometry using the *Join Geometry* node. The data contained in *Transform* output is not visible geometry, but just internal information that helps Blender to give a better user experience when using gizmos.
![image](/attachments/bc777d62-1ae3-47dc-afbd-0e31b86b8989)
The gizmo nodes have a multi-input socket. This allows **controlling multiple values** with the same gizmo.
Only a small set of **gizmo shapes** is supported initially. It might be extended in the future but one goal is to give the gizmos used by different node group assets a familiar look and feel. A similar constraint exists for **colors**. Currently, one can choose from a fixed set of colors which can be modified in the theme settings.
The set of **visible gizmos** is determined by a multiple factors because it's not really feasible to show all possible gizmos at all times. To see any of the geometry nodes gizmos, the "Active Modifier" option has to be enabled in the "Viewport Gizmos" popover. Then all gizmos are drawn for which at least one of the following is true:
* The gizmo controls an input of the active modifier of the active object.
* The gizmo controls a value in a selected node in an open node editor.
* The gizmo controls a pinned value in an open node editor. Pinning works by clicking the gizmo icon next to the value.
![image](/attachments/cdb8196e-ee03-499b-99d9-aba60ca73391)
Potential future improvements:
* More gizmo nodes.
* More inverse implementations of nodes.
* More fine grained control over which gizmos are shown.
* Support propagating gizmos back to attributes.
* Support propagating gizmos through drivers.
* Add screen space and 2D gizmos to transform gizmo.
* Support holding down the `alt` key to ignore soft limits.
* Add gizmos to some built-in nodes.
Links:
* Workshop 2023: https://code.blender.org/2023/11/geometry-nodes-workshop-november-2023/#gizmos
* Workshop 2024: https://devtalk.blender.org/t/2024-05-13-geometry-nodes-workshop-notes/34760#gizmos-3
* Feedback Thread: https://devtalk.blender.org/t/geometry-nodes-gizmos-feedback/32073
----
Some more impressions:
![image](/attachments/3c6275f2-1000-4539-9e99-10473f88b97a)
![image](/attachments/83ce7680-dc35-43c1-8292-b93f6d535a4a)
Somehow by setting the direction to zero and then some other value, I can create many gizmos from a single gizmo node. Even deleting the node or setting another active object doesn't remove them! They still move and control the value, but they don't reset to their previous position after interaction.
Scaling the object makes gizmos smaller and larger. Feels weird when everything else is sized in screen space
Remaining design topics IMO:
Node naming
Scaling of gizmos
From discussion in chat: a screen space option with a factor to scale from the default, and a "object space" option.
Node tools
I think it would be good to have an idea of how these would be used for "active node tools" before we commit it, just to be safe. I think once you have an active tool selected that corresponds to a node tool, the gizmo nodes would be evaluated and displayed, and you could use them to start the operation. But I think you'd have a better idea of the feasibility of something like that.
Overall I found the code pretty nice. My one larger concern is the growing extent to which we're adding specific code in places that are just general for the entire node system. I didn't get to the gizmo propagation code yet.
- [x] Somehow by setting the direction to zero and then some other value, I can create many gizmos from a single gizmo node. Even deleting the node or setting another active object doesn't remove them! They still move and control the value, but they don't reset to their previous position after interaction.
![image](/attachments/23bfa01b-1a69-44f5-ae34-66b66c3a965b)
- [x] Scaling the object makes gizmos smaller and larger. Feels weird when everything else is sized in screen space
![image](/attachments/90d0ea63-0c12-493c-ada6-6c2c822ca06b)
Remaining design topics IMO:
- [x] Node naming
- [x] Scaling of gizmos
- From discussion in chat: a screen space option with a factor to scale from the default, and a "object space" option.
- [x] Node tools
- I think it would be good to have an idea of how these would be used for "active node tools" before we commit it, just to be safe. I think once you have an active tool selected that corresponds to a node tool, the gizmo nodes would be evaluated and displayed, and you could use them to start the operation. But I think you'd have a better idea of the feasibility of something like that.
Overall I found the code pretty nice. My one larger concern is the growing extent to which we're adding specific code in places that are just general for the entire node system. I didn't get to the gizmo propagation code yet.
BKE_geometry_set.hh is a fairly common header, it would be nice to avoid the need to include these two headers here. Putting them in a separate class / header like CurvesEditHints and GreasePencilEditHints would make sense I think.
`BKE_geometry_set.hh` is a fairly common header, it would be nice to avoid the need to include these two headers here. Putting them in a separate class / header like `CurvesEditHints` and `GreasePencilEditHints` would make sense I think.
I don't really expect anything to change in this PR necessarily, but I find it really ugly how we keep adding these geometry nodes specific things into the base node tree runtime class. I think it adds confusion about the responsibility of different code. It would be better to generalize this a bit. But maybe that depends on our end goal for what node tree types are supposed to mean. Just deserves some active thought IMO.
I don't really expect anything to change in this PR necessarily, but I find it really ugly how we keep adding these geometry nodes specific things into the base node tree runtime class. I think it adds confusion about the responsibility of different code. It would be better to generalize this a bit. But maybe that depends on our end goal for what node tree types are supposed to mean. Just deserves some active thought IMO.
Hm, I don't really have a problem with it in this specific case. I haven't found a case where it actually hurts us (at least in run-time data). We could move the geometry nodes specific stuff to a separate struct, but it's not clear to me whether that makes the code better or just more verbose.
Hm, I don't really have a problem with it in this specific case. I haven't found a case where it actually hurts us (at least in run-time data). We could move the geometry nodes specific stuff to a separate struct, but it's not clear to me whether that makes the code better or just more verbose.
Storing this specific data in general structs is a serious anti-pattern IMO. 99% of the time this isn't needed, and it isn't related to anything else here.
Maybe it would work to store something in GizmoPropagationResult instead. Maybe a BitVector or a Set<int>
Storing this specific data in general structs is a serious anti-pattern IMO. 99% of the time this isn't needed, and it isn't related to anything else here.
Maybe it would work to store something in `GizmoPropagationResult` instead. Maybe a `BitVector` or a `Set<int>`
Last time we tried something like that with location, it didn't work out. Not sure if that situation has really changed. If it did, I'm fine with moving this data to a separate array, but I'm not willing to risk that as part of this patch. We can try that separately again.
Last time we tried something like that with `location`, it didn't work out. Not sure if that situation has really changed. If it did, I'm fine with moving this data to a separate array, but I'm not willing to risk that as part of this patch. We can try that separately again.
I guess this is covered by my other comments, but I find myself skeptical of the gizmo checks creeping in this far, running even in shader nodes, for example. It just seems like a hack. Some alternatives I could think of:
Use input sockets for these "input nodes" so this can be done automatically
Use a different "Value" input node for geometry nodes so this doesn't end up in every node system
Generalize a "extra button" callback that can be per node tree type
For generic socket drawing it's probably worth generalizing this more as well
I guess this is covered by my other comments, but I find myself skeptical of the gizmo checks creeping in this far, running even in shader nodes, for example. It just seems like a hack. Some alternatives I could think of:
- Use input sockets for these "input nodes" so this can be done automatically
- Use a different "Value" input node for geometry nodes so this doesn't end up in every node system
- Generalize a "extra button" callback that can be per node tree type
For generic socket drawing it's probably worth generalizing this more as well
I think on a conceptual level, an "extra button" callback for nodes and sockets would make sense here. However, it seems like this would not actually give us the control we want. For a better looking ui it's probably better to define how it's drawn in one place instead of two, especially because the ui elements belong closely together.
I see this somewhat similar to property decoration for creating keyframes.
With this additional callback, it wouldn't be so clear whether you should pass in layout or row, both are constraining in different ways.
I think on a conceptual level, an "extra button" callback for nodes and sockets would make sense here. However, it seems like this would not actually give us the control we want. For a better looking ui it's probably better to define how it's drawn in one place instead of two, especially because the ui elements belong closely together.
I see this somewhat similar to property decoration for creating keyframes.
With this additional callback, it wouldn't be so clear whether you should pass in `layout` or `row`, both are constraining in different ways.
Removed it for now. Couldn't reproduce the purpose anymore, but not sure if I really fixed it already or just don't know how to reproduce the issue anymore. If we'll need it again, it can also use DEG_id_tag_update_for_side_effect_request.
Removed it for now. Couldn't reproduce the purpose anymore, but not sure if I really fixed it already or just don't know how to reproduce the issue anymore. If we'll need it again, it can also use `DEG_id_tag_update_for_side_effect_request`.
switch (mode) {
case NODE_MATH_MULTIPLY:
return safe_divide(target_factor, second_value);
case NODE_MATH_DIVIDE:
return target_factor * second_value;
case NODE_MATH_RADIANS:
return RAD2DEG(target_factor);
case NODE_MATH_DEGREES:
return DEG2RAD(target_factor);
default:
return std::nullopt;
}
Looks so nice without the brackets :D
```
switch (mode) {
case NODE_MATH_MULTIPLY:
return safe_divide(target_factor, second_value);
case NODE_MATH_DIVIDE:
return target_factor * second_value;
case NODE_MATH_RADIANS:
return RAD2DEG(target_factor);
case NODE_MATH_DEGREES:
return DEG2RAD(target_factor);
default:
return std::nullopt;
}
```
Note sure about the existing nodes, but I could imagine that we could add nodes that allow deforming the transforms. It's quite similar to what we talked about for crazy space for curves.
Note sure about the existing nodes, but I could imagine that we could add nodes that allow deforming the transforms. It's quite similar to what we talked about for crazy space for curves.
JacquesLucke marked this conversation as resolved
Jacques Lucke
added 2 commits 2023-11-22 15:47:47 +01:00
I do after multiple considerations feel like there should be some (subtle) difference in drawing for the links that propagate the gizmo control backwards in addition to the icons on the sockets that do so.
When working on an example setup I found multiple instances where it gets confusing quite quickly how the gizmo control is flowing in the graph. The icons do can in theory communicate the complete information, but practically they are not sufficient since, when working on a node-tree, you don't always have all nodes in your view at all times, nor should you have to, to understand the information presented.
Case where the icons right now are not sufficient:
Is X and Y or either one used for the gizmo control?
Case where it's ambiguous which connection controls the group input:
Is one connection a gizmo or both? Which one is it?
And this might look like a stupid example, but having to scan all the connected nodes between on or potentially multiple reroutes that don't even have an icon themselves is a lot of overhead:
And again, I realize that only showing a part of the node-tree is not looking at all the information it provides, but understanding the dataflow should ideally not rely on seeing all the context. That's a big readability issue imo, that we are already fighting with the evaluation of fields, where you need to scan a lot of the node-tree to understand what is happening, rather than having all that context locally visible.
In a practical sense, looking at all the connected nodes first, which might be quite far away is quite inconvenient when you need this information to understand the context of the gizmo icon in the node.
One question that might alleviate the issue of presenting potentially irrelevant information and making the node-tree more noisy that it might need to be:
Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now. The icons would still be there. But then it would be more like showing the flow when it matters. Just an idea.
I do after multiple considerations feel like there should be some (subtle) difference in drawing for the links that propagate the gizmo control backwards in addition to the icons on the sockets that do so.
When working on an example setup I found multiple instances where it gets confusing quite quickly how the gizmo control is flowing in the graph. The icons ~~do~~ can in theory communicate the complete information, but practically they are not sufficient since, when working on a node-tree, you don't always have all nodes in your view at all times, nor should you have to, to understand the information presented.
Case where the icons right now are not sufficient:
![image](/attachments/f9258605-d044-4f61-9ea5-e0665d75541b)
Is X and Y or either one used for the gizmo control?
Case where it's ambiguous which connection controls the group input:
![image](/attachments/5d29c272-d92d-49e3-a88e-25d831fa5926)
Is one connection a gizmo or both? Which one is it?
And this might look like a stupid example, but having to scan all the connected nodes between on or potentially multiple reroutes that don't even have an icon themselves is a lot of overhead:
![image](/attachments/d0b71fe9-afb6-4d22-a79f-f751d7ddd412)
And again, I realize that only showing a part of the node-tree is not looking at all the information it provides, but understanding the dataflow should ideally not rely on seeing all the context. That's a big readability issue imo, that we are already fighting with the evaluation of fields, where you need to scan a lot of the node-tree to understand what is happening, rather than having all that context locally visible.
In a practical sense, looking at all the connected nodes first, which might be quite far away is quite inconvenient when you need this information to understand the context of the gizmo icon in the node.
One question that might alleviate the issue of presenting potentially irrelevant information and making the node-tree more noisy that it might need to be:
Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now. The icons would still be there. But then it would be more like showing the flow when it matters. Just an idea.
I wonder, how would knowing which links in your examples propagate a gizmo affect what you do? Knowing where a link goes helps of course, but that doesn't seem unique to gizmos at a first glance.
Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now.
Seems possible, but I wonder if that is good enough. It doesn't seem to help in the example you showed if the gizmo isn't visible already.
I wonder, how would knowing which links in your examples propagate a gizmo affect what you do? Knowing where a link goes helps of course, but that doesn't seem unique to gizmos at a first glance.
> Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now.
Seems possible, but I wonder if that is good enough. It doesn't seem to help in the example you showed if the gizmo isn't visible already.
I wonder, how would knowing which links in your examples propagate a gizmo affect what you do? Knowing where a link goes helps of course, but that doesn't seem unique to gizmos at a first glance.
I mean, the fact that you have to understand how the control of a gizmo is propagated back is not in question, is it? It's pretty important when setting up how the node-tree is being controlled to understand these connections. It's pretty important when connecting the a node link to see that this also means it will be controlled by a gizmo, potentially a second or third gizmo if you are connecting the value in multiple places. It's not so much changing my actions in how I connect the nodes, but more so giving me clear insight into the implications of my actions.
As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
Scanning every connected node with your eyes to find the string of connected nodes with the icon is not a valid replacement for making this clearly readable imo.
Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now.
Seems possible, but I wonder if that is good enough. It doesn't seem to help in the example you showed if the gizmo isn't visible already.
Exactly, my thought process was that in the cases that the gizmo isn't visible, you usually do not care about the propagation. Changing the drawing of the connected links that propagate the gizmo information when you select a node and makes its gizmos visible would also highlight that the gizmos that are now showing up will propagate changes in your node-tree.
> I wonder, how would knowing which links in your examples propagate a gizmo affect what you do? Knowing where a link goes helps of course, but that doesn't seem unique to gizmos at a first glance.
I mean, the fact that you have to understand how the control of a gizmo is propagated back is not in question, is it? It's pretty important when setting up how the node-tree is being controlled to understand these connections. It's pretty important when connecting the a node link to see that this also means it will be controlled by a gizmo, potentially a second or third gizmo if you are connecting the value in multiple places. It's not so much changing my actions in how I connect the nodes, but more so giving me clear insight into the implications of my actions.
As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
![image](/attachments/2f70835a-e02c-46fc-bff1-26db050a2adf)
Scanning every connected node with your eyes to find the string of connected nodes with the icon is not a valid replacement for making this clearly readable imo.
>
> > Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now.
>
> Seems possible, but I wonder if that is good enough. It doesn't seem to help in the example you showed if the gizmo isn't visible already.
Exactly, my thought process was that in the cases that the gizmo isn't visible, you usually do not care about the propagation. Changing the drawing of the connected links that propagate the gizmo information when you select a node and makes its gizmos visible would also highlight that the gizmos that are now showing up will propagate changes in your node-tree.
As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
That part could also be indicated by graying out the gizmo icon when the gizmo is "broken" because it's controlled in a way that does not support a gizmo I think.
Exactly, my thought process was that in the cases that the gizmo isn't visible, you usually do not care about the propagation. Changing the drawing of the connected links that propagate the gizmo information when you select a node and makes its gizmos visible would also highlight that the gizmos that are now showing up will propagate changes in your node-tree.
Hm ok, I can give this a try.
> As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
That part could also be indicated by graying out the gizmo icon when the gizmo is "broken" because it's controlled in a way that does not support a gizmo I think.
![image](/attachments/8a97a012-8763-4842-afc0-e5b0f2f2f7be)
> Exactly, my thought process was that in the cases that the gizmo isn't visible, you usually do not care about the propagation. Changing the drawing of the connected links that propagate the gizmo information when you select a node and makes its gizmos visible would also highlight that the gizmos that are now showing up will propagate changes in your node-tree.
Hm ok, I can give this a try.
As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
That part could also be indicated by graying out the gizmo icon when the gizmo is "broken" because it's controlled in a way that does not support a gizmo I think.
That's a good idea, but it's not actually what I meant. I just meant the totally valid case that I've shown in the screenshot. I think it's a lot more clear that the gizmos will have an effect on other values in your node-tree that the active node is connected to when the links indicate the flow of that control.
Another example would be wanting to manually control the value precisely, that you just set by eye in the viewport. Finding the actual value that has been modified is a lot easier if you can visually track the connections.
> > As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
>
> That part could also be indicated by graying out the gizmo icon when the gizmo is "broken" because it's controlled in a way that does not support a gizmo I think.
That's a good idea, but it's not actually what I meant. I just meant the totally valid case that I've shown in the screenshot. I think it's a lot more clear that the gizmos will have an effect on other values in your node-tree that the active node is connected to when the links indicate the flow of that control.
Another example would be wanting to manually control the value precisely, that you just set by eye in the viewport. Finding the actual value that has been modified is a lot easier if you can visually track the connections.
Jacques Lucke
added 1 commit 2023-12-16 14:06:56 +01:00
Jacques Lucke
changed title from WIP: Geometry Nodes: add support for gizmos to WIP: Geometry Nodes: attaching attaching gizmos for input values2023-12-22 23:18:46 +01:00
Jacques Lucke
changed title from WIP: Geometry Nodes: attaching attaching gizmos for input values to WIP: Geometry Nodes: support attaching gizmos for input values2023-12-22 23:21:35 +01:00
Jacques Lucke
changed title from WIP: Geometry Nodes: support attaching gizmos for input values to WIP: Geometry Nodes: support attaching gizmos to input values2023-12-22 23:21:46 +01:00
Jacques Lucke
added 3 commits 2023-12-23 00:13:46 +01:00
I think it would be good to have an idea of how these would be used for "active node tools" before we commit it, just to be safe. I think once you have an active tool selected that corresponds to a node tool, the gizmo nodes would be evaluated and displayed, and you could use them to start the operation. But I think you'd have a better idea of the feasibility of something like that.
That seems feasible. Will require a bit of refactoring, but probably not too much. Evaluating just the gizmo nodes that control operators inputs, without computing everything else is possible. Not sure if it will be required to disable that behavior in some cases since it has the potential to be slow.
> I think it would be good to have an idea of how these would be used for "active node tools" before we commit it, just to be safe. I think once you have an active tool selected that corresponds to a node tool, the gizmo nodes would be evaluated and displayed, and you could use them to start the operation. But I think you'd have a better idea of the feasibility of something like that.
That seems feasible. Will require a bit of refactoring, but probably not too much. Evaluating just the gizmo nodes that control operators inputs, without computing everything else is possible. Not sure if it will be required to disable that behavior in some cases since it has the potential to be slow.
Jacques Lucke
added 3 commits 2023-12-23 01:09:12 +01:00
Could be something I fixed already, but maybe the problem will also show up again later.
I noticed that this could use `DEG_id_tag_update_for_side_effect_request` nowadays if required.
Jacques Lucke
changed title from WIP: Geometry Nodes: support attaching gizmos to input values to Geometry Nodes: support attaching gizmos to input values2023-12-23 01:24:15 +01:00
Jacques Lucke
requested review from Hans Goudey 2023-12-23 01:24:16 +01:00
Only blender organization members with write access can start builds. See documentation for details.
Only blender organization members with write access can start builds. See [documentation](https://projects.blender.org/infrastructure/blender-bot/src/branch/main/README.md) for details.
I still have to read the inverse evaluation code properly. Just checking in on this branch again. It still works nicely from a UI perspective (though I won't comment on the link drawing :P)
I still have to read the inverse evaluation code properly. Just checking in on this branch again. It still works nicely from a UI perspective (though I won't comment on the link drawing :P)
I know I mentioned this before, but I still think putting geometry nodes specific things in the general bNode and bNodeSocket structs is problematic. Nowadays we don't reorder nodes based on selection, so can this move to a large boolean array or some set maybe? Same with the other.
I know I mentioned this before, but I still think putting geometry nodes specific things in the general `bNode` and `bNodeSocket` structs is problematic. Nowadays we don't reorder nodes based on selection, so can this move to a large boolean array or some set maybe? Same with the other.
Property descriptions for booleans should describe what happens when the value is true. So anything like "If true", "When enabled", or "when checked" should be avoided.
Property descriptions for booleans should describe what happens when the value is true. So anything like "If true", "When enabled", or "when checked" should be avoided.
Just a thought, do you think an alternative here is putting the gizmo "apply" code into an operator, more like existing gizmos like transform? Then the operator could do its own undo push. My guess is that would be a more "standard" use of Blender's architecture.
Just a thought, do you think an alternative here is putting the gizmo "apply" code into an operator, more like existing gizmos like transform? Then the operator could do its own undo push. My guess is that would be a more "standard" use of Blender's architecture.
Jacques Lucke
added 1 commit 2024-06-18 13:30:03 +02:00
I think the undo step is not working properly (?) When I make a change in the nodes and use gizmos multiple times it undoes all those things in one step
When a part of the node-tree is not evaluated/ not connected to the output, its gizmos can still be drawn and interacted with. But when using the transform output of the gizmos, that means that the transformation is not done according to the rest of the nodes. This is mainly a problem with the viewer node, especially combined with gizmo pinning. I think it might just be best to not draw these gizmos in the first place, since the geometry they are related to is also not drawn. I'm not actually sure this can be done reliably, but I guess it could just be done specifically when the transform output is used, since then the evaluation is clear. What do you think?
The primary + secondary link drawing actually works reasonably well for me visually. The thickness of the outline and the primary link should still be matched with the regular ones. I didn't do that properly in the mockup.
The aliasing is indeed quite noticeable when zooming out, unfortunately
There is an issue with terminating the back-propagation of the transform gizmo for individual components of it. This might be a restriction of how this works, but just thought I'd point it out, since this works when all components are invalid.
Just briefly checked the latest state again:
- I think the undo step is not working properly (?) When I make a change in the nodes and use gizmos multiple times it undoes all those things in one step
- When a part of the node-tree is not evaluated/ not connected to the output, its gizmos can still be drawn and interacted with. But when using the transform output of the gizmos, that means that the transformation is not done according to the rest of the nodes. This is mainly a problem with the viewer node, especially combined with gizmo pinning. I think it might just be best to not draw these gizmos in the first place, since the geometry they are related to is also not drawn. I'm not actually sure this can be done reliably, but I guess it could just be done specifically when the transform output is used, since then the evaluation is clear. What do you think?
- The primary + secondary link drawing actually works reasonably well for me visually. The thickness of the outline and the primary link should still be matched with the regular ones. I didn't do that properly in the mockup.
- The aliasing is indeed quite noticeable when zooming out, unfortunately
- There is an issue with terminating the back-propagation of the transform gizmo for individual components of it. This might be a restriction of how this works, but just thought I'd point it out, since this works when all components are invalid.
![image](/attachments/4c5f0719-304c-4417-9816-e958e790d194)
The transform gizmo node can now automatically disable e.g. all translation gizmos if the translation is not editable.
The new link drawing is now only done for active gizmos.
Removed support for propagating through drivers for now. Can be added back later if necessary.
If the transform output is used, only show the gizmos if the transforms have been propagated to the visible geometry.
I think I your feedback. Some new things:
* The transform gizmo node can now automatically disable e.g. all translation gizmos if the translation is not editable.
* The new link drawing is now only done for active gizmos.
* Removed support for propagating through drivers for now. Can be added back later if necessary.
* If the transform output is used, only show the gizmos if the transforms have been propagated to the visible geometry.
Thanks for implementing the context dependant link drawing depending on the gizmo visibility. I'm quite unsure about that. I think I'll have to do some more practical testing to figure out what I would prefer. I do think it can work, but there might also be some issues left with controlling better what gizmos are actually shown. (E.g. a way to pin all gizmos of a node-group in one click)
I also feel like this case would warrant showing the gizmo for the Offset input, when the node-group is active, even though it's being propagated to the vector input node. Right now it only shows when selecting the vector input node.
But I'm not sure about that, at least it would make sense to me to display the gizmo link connected to that node imo.
(Actually, I found a bug in the case of the screenshot: while the link drawing works reliably when selecting the vector node, the actual gizmo itself only works after re-evaluation of the node-tree, it seems. Specifically when making the node-group active first and then the vector node.)
But I think this is part of what I need to experience a bit more in practice and it would be good to get some more opinions on what to show when as well.
Removed support for propagating through drivers for now. Can be added back later if necessary.
Do you want to create a new task for that?
The drawing looks much nicer now!
Thanks for implementing the context dependant link drawing depending on the gizmo visibility. I'm quite unsure about that. I think I'll have to do some more practical testing to figure out what I would prefer. I do think it can work, but there might also be some issues left with controlling better what gizmos are actually shown. (E.g. a way to pin all gizmos of a node-group in one click)
![image](/attachments/35862bb6-1e7e-448a-b7e5-bd55215f850c)
I also feel like this case would warrant showing the gizmo for the `Offset` input, when the node-group is active, even though it's being propagated to the vector input node. Right now it only shows when selecting the vector input node.
But I'm not sure about that, at least it would make sense to me to display the gizmo link connected to that node imo.
(Actually, I found a bug in the case of the screenshot: while the link drawing works reliably when selecting the vector node, the actual gizmo itself only works after re-evaluation of the node-tree, it seems. Specifically when making the node-group active first and then the vector node.)
But I think this is part of what I need to experience a bit more in practice and it would be good to get some more opinions on what to show when as well.
> Removed support for propagating through drivers for now. Can be added back later if necessary.
Do you want to create a new task for that?
Thanks for implementing the context dependant link drawing depending on the gizmo visibility. I'm quite unsure about that.
I quite like it currently. I'm just not entirely sure if we can strictly couple it to the visibility in the viewport(s). E.g. sometimes gizmos are not shown there because they are disabled in the overlay settings, or because their transform is not propagated to the final geometry among other reasons. Taking that into account in node editor drawing is probably possible, but not entirely straight forward.
Actually, I found a bug in the case of the screenshot: while the link drawing works reliably when selecting the vector node, the actual gizmo itself only works after re-evaluation of the node-tree, it seems. Specifically when making the node-group active first and then the vector node.
Please provide a .blend file that has the bug. I can't reproduce it right now.
Do you want to create a new task for that?
I'll add it as a follow up task to the PR description. We can create a task for it a little later when we are ready here.
> Thanks for implementing the context dependant link drawing depending on the gizmo visibility. I'm quite unsure about that.
I quite like it currently. I'm just not entirely sure if we can strictly couple it to the visibility in the viewport(s). E.g. sometimes gizmos are not shown there because they are disabled in the overlay settings, or because their transform is not propagated to the final geometry among other reasons. Taking that into account in node editor drawing is probably possible, but not entirely straight forward.
> Actually, I found a bug in the case of the screenshot: while the link drawing works reliably when selecting the vector node, the actual gizmo itself only works after re-evaluation of the node-tree, it seems. Specifically when making the node-group active first and then the vector node.
Please provide a .blend file that has the bug. I can't reproduce it right now.
> Do you want to create a new task for that?
I'll add it as a follow up task to the PR description. We can create a task for it a little later when we are ready here.
Jacques Lucke
added 2 commits 2024-07-01 08:56:04 +02:00
> Please provide a .blend file that has the bug. I can't reproduce it right now.
[procedural_modeling-gizmo_test.blend](/attachments/89186548-7765-4404-92a4-49b310916276)
- click on vector node
- click on node-group
- click on vector node again
For the record, it seems this gizmo implementation is separate enough from geometry nodes evaluation and only relation is to know which nodes should be evaluated by following gizmos logic.
So, probably this will be easy to support gizmo in other backends like composition or texture nodes. By easy i mean that there is no need to rewrite 100% of the code just to evaluate values.
Next one point is that this is not just another one suga for funny video with using gizmo for control resolution of the circular staircase in the house. This is the step in direction to replace python (and not only) operators by node tools with now gizmo settings.
For the record, it seems this gizmo implementation is separate enough from geometry nodes evaluation and only relation is to know which nodes should be evaluated by following gizmos logic.
So, probably this will be easy to support gizmo in other backends like composition or texture nodes. By _easy_ i mean that there is no need to rewrite 100% of the code just to evaluate values.
Next one point is that this is not just another one suga for funny video with using gizmo for control resolution of the circular staircase in the house. This is the step in direction to replace python (and not only) operators by node tools with now gizmo settings.
Jacques Lucke
added 1 commit 2024-07-03 16:10:41 +02:00
This looks quite good to me. I'm happy with the different levels of abstraction and how easy it was to understand the code. I didn't read deeply enough to fully understand all the details but I'm was satisfied that anyone could as necessary.
One thing that would have been helpful in my engineering visualization files where I was testing adding gizmos is a circle drawing mode for the linear gizmo:
It would also be very helpful to click on the gizmos to enter values directly. At that point I think I could give these files to people not familiar with Blender and they would be able to do compelling things. But that's a more general gizmo feature I guess.
I find the link drawing quite nice and I like the context dependent drawing as well, since it mirrors what you see in the 3D view. Overall, big +1, I think this is ready to land!
This looks quite good to me. I'm happy with the different levels of abstraction and how easy it was to understand the code. I didn't read deeply enough to fully understand all the details but I'm was satisfied that anyone could as necessary.
One thing that would have been helpful in my engineering visualization files where I was testing adding gizmos is a circle drawing mode for the linear gizmo:
![image](/attachments/4af730e8-f40b-4178-b7e2-691926631b94)
It would also be very helpful to click on the gizmos to enter values directly. At that point I think I could give these files to people not familiar with Blender and they would be able to do compelling things. But that's a more general gizmo feature I guess.
I find the link drawing quite nice and I like the context dependent drawing as well, since it mirrors what you see in the 3D view. Overall, big +1, I think this is ready to land!
This adds support for attaching gizmos for input values. The goal is to make it easier for users to set input values intuitively in the 3D viewport.
We went through multiple different possible designs until we settled on the one implemented here. We picked it for it's flexibility and ease of use when using geometry node assets. The core principle in the design is that gizmos are attached to existing input values instead of being the input value themselves. This actually fits the existing concept of gizmos in Blender well, but may be a bit unintutitive in a node setup at first. The attachment is done using links in the node editor.
The most basic usage of the node is to link a Value node to the new Linear Gizmo node. This attaches the gizmo to the input value and allows you to change it from the 3D view. The attachment is indicated by the gizmo icon in the sockets which are controlled by a gizmo as well as the back-link (notice the double link) when the gizmo is active.
The core principle makes it straight forward to control the same node setup from the 3D view with gizmos, or by manually changing input values, or by driving the input values procedurally.
If the input value is controlled indirectly by other inputs, it's often possible to automatically propagate the gizmo to the actual input.
Backpropagation does not work for all nodes, although more nodes can be supported over time.
This patch adds the first three gizmo nodes which cover common use cases:
In the future, more built-in gizmos and potentially the ability for custom gizmos could be added.
All gizmo nodes have a Transform geometry output. Using it is optional but it is recommended when the gizmo is used to control inputs that affect a geometry. When it is used, Blender will automatically transform the gizmos together with the geometry that they control. To achieve this, the output should be merged with the generated geometry using the Join Geometry node. The data contained in Transform output is not visible geometry, but just internal information that helps Blender to give a better user experience when using gizmos.
The gizmo nodes have a multi-input socket. This allows controlling multiple values with the same gizmo.
Only a small set of gizmo shapes is supported initially. It might be extended in the future but one goal is to give the gizmos used by different node group assets a familiar look and feel. A similar constraint exists for colors. Currently, one can choose from a fixed set of colors which can be modified in the theme settings.
The set of visible gizmos is determined by a multiple factors because it's not really feasible to show all possible gizmos at all times. To see any of the geometry nodes gizmos, the "Active Modifier" option has to be enabled in the "Viewport Gizmos" popover. Then all gizmos are drawn for which at least one of the following is true:
Potential future improvements:
alt
key to ignore soft limits.Links:
Some more impressions:
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
Remaining design topics IMO:
Overall I found the code pretty nice. My one larger concern is the growing extent to which we're adding specific code in places that are just general for the entire node system. I didn't get to the gizmo propagation code yet.
@ -680,6 +691,7 @@ class NODE_MT_geometry_node_add_all(Menu):
layout.menu("NODE_MT_category_GEO_TEXTURE")
layout.menu("NODE_MT_category_GEO_UTILITIES")
layout.separator()
layout.menu("NODE_MT_category_GEO_GIZMO")
Might as well hide this for tools
@ -14,3 +16,4 @@
#include "BLI_bounds_types.hh"
#include "BLI_function_ref.hh"
#include "BLI_map.hh"
#include "BLI_math_matrix_types.hh"
BKE_geometry_set.hh
is a fairly common header, it would be nice to avoid the need to include these two headers here. Putting them in a separate class / header likeCurvesEditHints
andGreasePencilEditHints
would make sense I think.@ -133,6 +136,7 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
/** Information about usage of anonymous attributes within the group. */
std::unique_ptr<anonymous_attribute_inferencing::AnonymousAttributeInferencingResult>
anonymous_attribute_inferencing;
std::unique_ptr<nodes::gizmos::GizmoPropagationResult> gizmo_inferencing;
I don't really expect anything to change in this PR necessarily, but I find it really ugly how we keep adding these geometry nodes specific things into the base node tree runtime class. I think it adds confusion about the responsibility of different code. It would be better to generalize this a bit. But maybe that depends on our end goal for what node tree types are supposed to mean. Just deserves some active thought IMO.
Hm, I don't really have a problem with it in this specific case. I haven't found a case where it actually hurts us (at least in run-time data). We could move the geometry nodes specific stuff to a separate struct, but it's not clear to me whether that makes the code better or just more verbose.
@ -199,0 +204,4 @@
* This is set in #update_gizmo_propagation and is stored here so that it can be quickly accessed
* during socket drawing.
*/
bool has_gizmo = false;
Storing this specific data in general structs is a serious anti-pattern IMO. 99% of the time this isn't needed, and it isn't related to anything else here.
Maybe it would work to store something in
GizmoPropagationResult
instead. Maybe aBitVector
or aSet<int>
Last time we tried something like that with
location
, it didn't work out. Not sure if that situation has really changed. If it did, I'm fine with moving this data to a separate array, but I'm not willing to risk that as part of this patch. We can try that separately again.@ -97,1 +97,3 @@
uiItemR(layout, &sockptr, "default_value", DEFAULT_FLAGS, "", ICON_NONE);
uiLayout *row = uiLayoutRow(layout, true);
uiItemR(row, &sockptr, "default_value", DEFAULT_FLAGS, "", ICON_NONE);
if (output->runtime->has_gizmo) {
I guess this is covered by my other comments, but I find myself skeptical of the gizmo checks creeping in this far, running even in shader nodes, for example. It just seems like a hack. Some alternatives I could think of:
For generic socket drawing it's probably worth generalizing this more as well
I think on a conceptual level, an "extra button" callback for nodes and sockets would make sense here. However, it seems like this would not actually give us the control we want. For a better looking ui it's probably better to define how it's drawn in one place instead of two, especially because the ui elements belong closely together.
I see this somewhat similar to property decoration for creating keyframes.
With this additional callback, it wouldn't be so clear whether you should pass in
layout
orrow
, both are constraining in different ways.@ -0,0 +76,4 @@
if (prop_type == PROP_FLOAT) {
return RNA_property_float_get_index(&this->owner, this->property, *this->index);
}
else if (prop_type == PROP_INT) {
Else after return
@ -0,0 +137,4 @@
RNA_property_update(C, &this->owner, this->property);
if (Object *object = CTX_data_active_object(C)) {
/* The recalc is necessary so that the gizmo positions are updated even if they don't affect
* the final output currently. */
Do you have an idea for how this could be fixed? Maybe just tagging the region for redraw could work?
Removed it for now. Couldn't reproduce the purpose anymore, but not sure if I really fixed it already or just don't know how to reproduce the issue anymore. If we'll need it again, it can also use
DEG_id_tag_update_for_side_effect_request
.@ -0,0 +158,4 @@
const bNodeSocket &second_input_socket = node.input_socket(1);
const float second_value =
tree_log.find_primitive_socket_value<float>(second_input_socket).value_or(0.0f);
switch (mode) {
Looks so nice without the brackets :D
@ -253,1 +253,4 @@
}
if (geometry.get_component<GeometryComponentEditData>()) {
auto &component = geometry.get_component_for_write<GeometryComponentEditData>();
for (float4x4 &m : component.gizmo_transforms_.values()) {
Just curious, do you imagine that other nodes would end up being able to deform the gizmo transforms?
Note sure about the existing nodes, but I could imagine that we could add nodes that allow deforming the transforms. It's quite similar to what we talked about for crazy space for curves.
@blender-bot package
Package build started. Download here when ready.
I do after multiple considerations feel like there should be some (subtle) difference in drawing for the links that propagate the gizmo control backwards in addition to the icons on the sockets that do so.
When working on an example setup I found multiple instances where it gets confusing quite quickly how the gizmo control is flowing in the graph. The icons
docan in theory communicate the complete information, but practically they are not sufficient since, when working on a node-tree, you don't always have all nodes in your view at all times, nor should you have to, to understand the information presented.Case where the icons right now are not sufficient:
Is X and Y or either one used for the gizmo control?
Case where it's ambiguous which connection controls the group input:
Is one connection a gizmo or both? Which one is it?
And this might look like a stupid example, but having to scan all the connected nodes between on or potentially multiple reroutes that don't even have an icon themselves is a lot of overhead:
And again, I realize that only showing a part of the node-tree is not looking at all the information it provides, but understanding the dataflow should ideally not rely on seeing all the context. That's a big readability issue imo, that we are already fighting with the evaluation of fields, where you need to scan a lot of the node-tree to understand what is happening, rather than having all that context locally visible.
In a practical sense, looking at all the connected nodes first, which might be quite far away is quite inconvenient when you need this information to understand the context of the gizmo icon in the node.
One question that might alleviate the issue of presenting potentially irrelevant information and making the node-tree more noisy that it might need to be:
Would it be possible to only draw the links differently whenever the gizmos are actually drawn? That way, in most of the cases where the gizmo back propagation is something you don't care about the node-tree wouldn't look any different then now. The icons would still be there. But then it would be more like showing the flow when it matters. Just an idea.
I wonder, how would knowing which links in your examples propagate a gizmo affect what you do? Knowing where a link goes helps of course, but that doesn't seem unique to gizmos at a first glance.
Seems possible, but I wonder if that is good enough. It doesn't seem to help in the example you showed if the gizmo isn't visible already.
I mean, the fact that you have to understand how the control of a gizmo is propagated back is not in question, is it? It's pretty important when setting up how the node-tree is being controlled to understand these connections. It's pretty important when connecting the a node link to see that this also means it will be controlled by a gizmo, potentially a second or third gizmo if you are connecting the value in multiple places. It's not so much changing my actions in how I connect the nodes, but more so giving me clear insight into the implications of my actions.
As an example, if I have a node in my node-tree that uses gizmos for the inputs and I connect something to one of these inputs, the gizmo is still usable to control the value that this link propagates the information to. This should be clearly visible from the node that shows this connection imo.
Scanning every connected node with your eyes to find the string of connected nodes with the icon is not a valid replacement for making this clearly readable imo.
Exactly, my thought process was that in the cases that the gizmo isn't visible, you usually do not care about the propagation. Changing the drawing of the connected links that propagate the gizmo information when you select a node and makes its gizmos visible would also highlight that the gizmos that are now showing up will propagate changes in your node-tree.
That part could also be indicated by graying out the gizmo icon when the gizmo is "broken" because it's controlled in a way that does not support a gizmo I think.
Hm ok, I can give this a try.
That's a good idea, but it's not actually what I meant. I just meant the totally valid case that I've shown in the screenshot. I think it's a lot more clear that the gizmos will have an effect on other values in your node-tree that the active node is connected to when the links indicate the flow of that control.
Another example would be wanting to manually control the value precisely, that you just set by eye in the viewport. Finding the actual value that has been modified is a lot easier if you can visually track the connections.
WIP: Geometry Nodes: add support for gizmosto WIP: Geometry Nodes: attaching attaching gizmos for input valuesWIP: Geometry Nodes: attaching attaching gizmos for input valuesto WIP: Geometry Nodes: support attaching gizmos for input valuesWIP: Geometry Nodes: support attaching gizmos for input valuesto WIP: Geometry Nodes: support attaching gizmos to input valuesThat seems feasible. Will require a bit of refactoring, but probably not too much. Evaluating just the gizmo nodes that control operators inputs, without computing everything else is possible. Not sure if it will be required to disable that behavior in some cases since it has the potential to be slow.
@blender-bot build
WIP: Geometry Nodes: support attaching gizmos to input valuesto Geometry Nodes: support attaching gizmos to input values@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
@blender-bot build
@blender-bot package
Package build started. Download here when ready.
Only blender organization members with write access can start builds. See documentation for details.
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
I still have to read the inverse evaluation code properly. Just checking in on this branch again. It still works nicely from a UI perspective (though I won't comment on the link drawing :P)
@ -354,6 +361,9 @@ struct bNodeType {
/** Get extra information that is drawn next to the node. */
NodeExtraInfoFunction get_extra_info;
NodeInverseElemEvalFunction eval_inverse_elem;
Would be nice to have some info about the responsibility of these callbacks here. Or a reference to some comments elsewhere.
@ -208,6 +212,8 @@ class bNodeSocketRuntime : NonCopyable, NonMovable {
*/
short total_inputs = 0;
bool has_gizmo = false;
I know I mentioned this before, but I still think putting geometry nodes specific things in the general
bNode
andbNodeSocket
structs is problematic. Nowadays we don't reorder nodes based on selection, so can this move to a large boolean array or some set maybe? Same with the other.@ -0,0 +23,4 @@
b.add_input<decl::Bool>("Screen Space")
.default_value(true)
.description(
"If true, true gizmo is displayed in screen space. Otherwise it's in object space");
Property descriptions for booleans should describe what happens when the value is true. So anything like "If true", "When enabled", or "when checked" should be avoided.
@ -467,6 +467,9 @@ static void gizmo_tweak_finish(bContext *C, wmOperator *op, const bool cancel, b
wm_gizmomap_modal_set(mtweak->gzmap, C, mtweak->gz_modal, nullptr, false);
}
}
if (mtweak->gz_modal->flag & WM_GIZMO_NEEDS_UNDO) {
Just a thought, do you think an alternative here is putting the gizmo "apply" code into an operator, more like existing gizmos like transform? Then the operator could do its own undo push. My guess is that would be a more "standard" use of Blender's architecture.
Just briefly checked the latest state again:
I think I your feedback. Some new things:
@blender-bot package
Package build started. Download here when ready.
The drawing looks much nicer now!
Thanks for implementing the context dependant link drawing depending on the gizmo visibility. I'm quite unsure about that. I think I'll have to do some more practical testing to figure out what I would prefer. I do think it can work, but there might also be some issues left with controlling better what gizmos are actually shown. (E.g. a way to pin all gizmos of a node-group in one click)
I also feel like this case would warrant showing the gizmo for the
Offset
input, when the node-group is active, even though it's being propagated to the vector input node. Right now it only shows when selecting the vector input node.But I'm not sure about that, at least it would make sense to me to display the gizmo link connected to that node imo.
(Actually, I found a bug in the case of the screenshot: while the link drawing works reliably when selecting the vector node, the actual gizmo itself only works after re-evaluation of the node-tree, it seems. Specifically when making the node-group active first and then the vector node.)
But I think this is part of what I need to experience a bit more in practice and it would be good to get some more opinions on what to show when as well.
Do you want to create a new task for that?
I quite like it currently. I'm just not entirely sure if we can strictly couple it to the visibility in the viewport(s). E.g. sometimes gizmos are not shown there because they are disabled in the overlay settings, or because their transform is not propagated to the final geometry among other reasons. Taking that into account in node editor drawing is probably possible, but not entirely straight forward.
Please provide a .blend file that has the bug. I can't reproduce it right now.
I'll add it as a follow up task to the PR description. We can create a task for it a little later when we are ready here.
procedural_modeling-gizmo_test.blend
@blender-bot package
Package build started. Download here when ready.
@blender-bot package
Package build started. Download here when ready.
For the record, it seems this gizmo implementation is separate enough from geometry nodes evaluation and only relation is to know which nodes should be evaluated by following gizmos logic.
So, probably this will be easy to support gizmo in other backends like composition or texture nodes. By easy i mean that there is no need to rewrite 100% of the code just to evaluate values.
Next one point is that this is not just another one suga for funny video with using gizmo for control resolution of the circular staircase in the house. This is the step in direction to replace python (and not only) operators by node tools with now gizmo settings.
This looks quite good to me. I'm happy with the different levels of abstraction and how easy it was to understand the code. I didn't read deeply enough to fully understand all the details but I'm was satisfied that anyone could as necessary.
One thing that would have been helpful in my engineering visualization files where I was testing adding gizmos is a circle drawing mode for the linear gizmo:
It would also be very helpful to click on the gizmos to enter values directly. At that point I think I could give these files to people not familiar with Blender and they would be able to do compelling things. But that's a more general gizmo feature I guess.
I find the link drawing quite nice and I like the context dependent drawing as well, since it mirrors what you see in the 3D view. Overall, big +1, I think this is ready to land!
@ -0,0 +19,4 @@
* the path of group nodes to get from the geometry nodes modifier to the group containing the
* gizmo node).
*/
struct GeoNodesGizmoID {
How about
NodeGizmoID
? Since there's nothing really specific to geometry nodes in this struct@ -1307,0 +1324,4 @@
FN_NODE_INPUT_INT,
FN_NODE_INPUT_BOOL,
FN_NODE_INPUT_ROTATION,
NODE_GROUP_INPUT))
Could you extract this node type ELEM check to a separate function?
@ -4165,0 +4175,4 @@
snode.edittree->ensure_topology_cache();
/* Compute compute context hash for the current node tree path. */
ComputeContextHash current_compute_context_hash;
What do you think about this?
Note that this is not semantically the same, because the failure case is not properly handled yet. Will use your code with added error handling.
In the end this is not constant variable, lambda now meaningless\
@ -27,8 +27,10 @@ struct NodesModifierRuntime {
/**
* Contains logged information from the last evaluation.
* This can be used to help the user to debug a node tree.
* This is a shared pointer, because we might want to keep it around in some cases after the
This is a shared pointer, because
->This is a shared pointer because