Page MenuHome

Geometry Nodes - Object Info: Transformation Space
ClosedPublic

Authored by Dalai Felinto (dfelinto) on Jan 15 2021, 7:26 PM.

Details

Summary

By design the modified object transformations should still work and
affect the geometry nodes results. The current behavior, however, make
the geometry from the object info node not affected by the modified
object transformations.

In a similar fashion the Location, Rotation and Scale sockets outputs should
be aware of whether the output should be in the global space or in the
space of the nodetree.

To solve this, the patch introduces a new transformation space "enum" where
users can pick "Local Space" or "No Transform" space.

No Transform space violates a bit the design of the nodetree. The geometry in this case
is transformed so that moving the modified object doesn't interfere with the geometry.
This is particularly useful for the boolean node for instance. In this case the
Location/Rotation/Scale are also outputted in global space.

Local Space is the default space. In this case Location/Rotation/Scale are provided
in relation to the modified object space. This is useful to check if an object is close to
vertices and act accordingly. The geometry in this case matches the obtained by the
point instance node.

This will be particularly important once the the point instance node gets a new input type
"Geometry". In that case plugging the geometry from the object info or using the object
input should yield the same results.

Old files have the space as "No Transform" by default to prevent any changes in behavior.


Bonus design consideration. The same behavior is expected for the collection
info node (T84603) in relation to the offset setting of the collection.

Diff Detail

Repository
rB Blender
Branch
temp-geometry-nodes-object-info-space (branched from master)
Build Status
Buildable 12214
Build 12214: arc lint + arc unit

Event Timeline

Dalai Felinto (dfelinto) requested review of this revision.Jan 15 2021, 7:26 PM

Some notes:

  • The code itself is from @Jacques Lucke (JacquesLucke)
  • The "review" here is regarding the design. I'm very strong about that but we agreed to run this by @Brecht Van Lommel (brecht).
  • It would be nice to let this sink in to the rest of the team as well since I seem to be the only one pushing for this change.
  • This patch is against master, but it would be actually applied to the 2.92 branch.

And of course level-headed feedback by artists is welcome as always.

TL;DR: I think it's ok to do this, but with an option on the Object Info node to transform into local space or not. Default to no transform.

There are two competing use cases.

  • For instancing or an array modifier (which is basically instancing too), typically the transform should be ignored.
  • For modeling modifiers like booleans, or rigging modifiers like mesh deform or shrink wrap, typically you want the transform to be applied since the relative positions of objects is needed.

Both are common, and so for convenience and discoverability I think the Object Info node should have an option. Then if the result is not what a user expects or forgets to apply the transform, there is some hint or reminder to find the solution. Default to not applying the transform seems ok.

The Points to Instances node will have modes for Geometry / Object / Collection. Will modifiers like boolean, mesh deform, shrink wrap all get a similar option to choose between Geometry and Object? And what will be the default mode? The default workflow there maybe affects the right choice for the default value on the Object Info node.

There's also a performance consideration. Ideally if you pull in Geometry from another object, it would not make a copy and even for instancing it should ideally still reference the original geometry as long as it has not be modified. For that not transforming by default is better.

An alternative solution would be to define a geometry set as having an associated transform matrix. It would then be up to each node to use this transform matrix, or not. A Points to Instances node would ignore it by default, while other nodes would mostly use it by default. This adds complexity in other places though, as you now need nodes to modify that transform and users need to be aware of it. So I'm not sure that's actually a good idea.

Adding "Apply Transforms" as an explicit option in the node

Missing from previous update

  • From feedback: change Loc/Rot/Sca
  • From Review: Apply Transforms > Apply Transform

The transformations should be also transformed or not depending on the new option.
Besides, the current result we have for them should not be the default since
it is not consistent with the geometry transforms.

If I understand this correctly and the checkbox converts local to world space, I'd advocate to naming the checkbox simply "Use World Space" because "Apply Transforms" has other meanings. I can see a world where user A asks for help regarding why their instances aren't moving with their objects, helpful user B tells them "Have you tried Apply Transforms?" to which user A goes in the 3D viewport and presses Ctrl+A and then is sad. ;)
(Also, since checkboxes should be on/off functionalities, this would ideally be an enum, but I guess in the context of nodes, enums are not so great?)

  • From review: Add an enum for the space type
Hans Goudey (HooglyBoogly) requested changes to this revision.Jan 18 2021, 4:49 PM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/blenloader/intern/versioning_290.c
1559–1573 ↗(On Diff #32872)

I think this change shouldn't be in here?

source/blender/makesrna/intern/rna_nodetree.c
498 ↗(On Diff #32872)

If there's no description I think this usually gets set to "" rather than NULL.

But this is a situation where we really need good tooltips. This stuff is not obvious and it's a perfect spot to have some information.

8863 ↗(On Diff #32872)

Same comment here, I think we need a description here, and ideally a more descriptive property name too.

8864 ↗(On Diff #32872)

I think "rna_Node_update" would work here

This revision now requires changes to proceed.Jan 18 2021, 4:49 PM

This needs a good tooltip, to explain why you might choose one or the other.

I don't think "Global" is a good name. Neither of the options transforms into or from global space.

The choice is between ignoring the transform and transforming into the local space.

Dalai Felinto (dfelinto) retitled this revision from Geometry Nodes - Object Info: don't apply the obj transform to the geometry to Geometry Nodes - Object Info: Transformation Space.Jan 18 2021, 5:03 PM
Dalai Felinto (dfelinto) edited the summary of this revision. (Show Details)

Thanks for taking the feedback Dalai!

These go into a bit more detail than the tooltips currently found on constraints and drivers, where similar enums exist. Maybe they break some convention on tooltip length? Definitely better than no tooltip! :)

source/blender/makesrna/intern/rna_nodetree.c
498 ↗(On Diff #32872)

Suggested names/tooltips, hoping they're not lies:
Local Space: Transformations relative to the object's local coordinate system. This includes posing and constraints but excludes transformations induced by parenting.
World Space: The object's final transformations relative to the world origin.

  • From Review: Some tooltips

(Also, since checkboxes should be on/off functionalities, this would ideally be an enum, but I guess in the context of nodes, enums are not so great?)

I do find it sad that turning it into an enum means losing the ability to plug a noodle in it. Admittedly, I don't see a use case, but what if there is one out there? :D

For that hypothetical future use-case, we should allow enums to have sockets, not compromise on the design here IMO.

Here are some suggestions for the tooltips:

Transform SpaceDetermine the transformations applied to the vector sockets and the geometry
No TransformUse the object's final transformations relative to the world origin, unaffected by changes to the modifier object's transform
Local SpaceUse the geometry's coordinates relative to the object's origin, affected by changes the modifier object's transform
  • From review: new names + tooltips
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/makesrna/intern/rna_nodetree.c
508 ↗(On Diff #32879)

Actually, mind moving this "s"?

8876 ↗(On Diff #32879)

Actually I think this is a bit better
"Determine the transformation applied to vector and geometry outputs"

This revision is now accepted and ready to land.Jan 18 2021, 7:11 PM
  • Final tooltip review

I will push this tomorrow. The merge in master will require more
attention than I can do it today.

So another day for anyone to change their mind (please dont).

  • Last round of name/tooltips: Original | Relative
  • Last iteration: tooltips by Brecht