Details

Reviewers

Maniphest Tasks

T91852: Output UVs from mesh primitive nodes with anonymous attributes

Commits

rBf879c20f72d9: Geometry Nodes: output uv map from primitive nodes as anonymous attributes

Summary

Currently, some mesh primitive nodes create a uv map with the hardcoded name uv_map. This is fairly non-standard in geometry nodes. Typically, we want to output generated attributes as sockets. This would also have the benefit that the attribute does not have to be computed when it is not used.

This patch outputs the uv map as a vector from the primitive nodes. Since the generated geometry now does not contain the uv_map node anymore, an additional Store Named Attribute node is added to existing nodes to restore the old behavior.

Additionally, this patch also adds support for 2D vectors in the Store Named Attribute node (this should probably be committed separately). Also see T92765: 2D Vector Socket Type. For now, 2d vectors are still 3d vectors within geometry nodes, but when storing the attribute, 2d vectors can be used to reduce memory consumption.

This commit breaks forward compatibility in the case when the uv map was used before.

Tests have to be updated afterwards, because an attribute that was 2d before is now 3d due to the versioning code.

Diff Detail

Repository

rB Blender

Branch

primitive-uv-maps (branched from master)

Build Status

Buildable 25035
Build 25035: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Nov 28 2022, 4:11 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B24845: Diff 58119.Nov 28 2022, 4:11 PM

Iliya Katueshenock (Moder) awarded a token.Nov 28 2022, 4:12 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Nov 28 2022, 4:17 PM

Jacques Lucke (JacquesLucke) added a reviewer: Hans Goudey (HooglyBoogly).

Jacques Lucke (JacquesLucke) added a task: T91852: Output UVs from mesh primitive nodes with anonymous attributes.Nov 28 2022, 4:21 PM

https://builder.blender.org/admin/#/builders/136/builds/256

A couple things to mention in the commit message:

I'd mention T92765, in combination with the "2D vectors as 3D vector sockets" point
Might be worth mentioning the performance aspect, since the UV map was an expensive part of some of the primitives

source/blender/blenkernel/intern/node.cc
1133	Why is this necessary?
source/blender/blenloader/intern/versioning_300.cc
858	In the past I found it a bit helpful to leave the selection in the versioning code. That way you could open a file and see the nodes that were added automatically more easy. Not a big deal either way though. Actually, I don't see where `nodeAddStaticNode` even selects the new node, hmm

Just requesting changes based on my previous comment.

This revision now requires changes to proceed.Dec 5 2022, 4:07 PM

Merge branch 'master' into primitive-uv-maps
fix
cleanup

Harbormaster completed remote builds in B25001: Diff 58507.Dec 12 2022, 12:26 PM

https://builder.blender.org/admin/#/builders/136/builds/279

source/blender/blenkernel/intern/node.cc
1133	Well, because the typeinfo may not be set in versioning code yet.

It seems like it is harder to add the new nodes behind others now, because the node order is also stored separately from the nodes linked-list. Maybe you have an idea, or maybe we don't care.

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Dec 12 2022, 12:32 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

It should be possible to add the nodes wherever you want and then rebuild the node vector:

// Add node somewhere in the ListBase
nodeUniqueID(node_tree, new_node); // Make sure the node has an identifier
nodeRebuildIDVector(node_tree); // Update the vector to match the list

Merge branch 'master' into primitive-uv-maps
move nodes to front
cleanup

Harbormaster completed remote builds in B25012: Diff 58541.Dec 13 2022, 11:14 AM

https://builder.blender.org/admin/#/builders/136/builds/282

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Dec 14 2022, 12:15 AM

The code is pretty simple here. Thanks for doing this.

Regarding the comment about renaming an attribute, maybe that can be a callback on MutableAttributeAccessor. And I see no reason why it shouldn't be a node too!

source/blender/blenloader/intern/versioning_300.cc
892	I don't find `auto` worth it here, since it doesn't get the whole thing on a single line. But it doesn't really matter ;)

This revision is now accepted and ready to land.Dec 14 2022, 1:24 AM

Regarding the comment about renaming an attribute, maybe that can be a callback on MutableAttributeAccessor. And I see no reason why it shouldn't be a node too!

Yes, that sounds reasonable.