Fix T94113: Local view + Geometry Nodes is broken for instances
ClosedPublic
Actions

Authored by Germano Cavalcante (mano-wii) on Dec 24 2021, 2:47 PM.

Details

Reviewers

Hans Goudey (HooglyBoogly)

Maniphest Tasks

T94113: Local view + Geometry Nodes is broken for instances

Commits

rB1c7d7c915063: Fix T94113: Local view + Geometry Nodes is broken for instances

Summary

GeometrySet::compute_boundbox_without_instances may not initialize min
max in some cases such as meshes without vertices.

This can result in a Bounding Box with impossible dimensions
(min=FLT_MAX, max=-FLT_MAX).

So repeat the same solution seen in BKE_object_boundbox_calc_from_mesh
and set boundbox values to zero.

Diff Detail

Repository

rB Blender

Branch

master

Build Status

Buildable 19616
Build 19616: arc lint + arc unit

Event Timeline

Germano Cavalcante (mano-wii) requested review of this revision.Dec 24 2021, 2:47 PM

Germano Cavalcante (mano-wii) created this revision.

Harbormaster completed remote builds in B19616: Diff 46360.Dec 24 2021, 2:47 PM

Germano Cavalcante (mano-wii) added a task: T94113: Local view + Geometry Nodes is broken for instances.Dec 24 2021, 3:10 PM

Looks like a good improvement. However, for consistency, I think BKE_pointcloud_minmax and curve->bounds_min_max should return false if they are empty (for curves, that would be if there were no splines or all splines had no evaluated points.

source/blender/blenkernel/intern/object.cc
3921–3922	Looking at the state of the code afterwards, this comment might as well be in the other function too, saying "follow the convention set in #BKE_object_boundbox_calc_from_evaluated_geometry". Since this comment documents the history of the code rather than the code itself, I'd suggest leaving it to the commit message.

This revision now requires changes to proceed.Dec 24 2021, 5:12 PM

Implement emptiness checking for curves and point cloud
Improve comments

Harbormaster completed remote builds in B19622: Diff 46367.Dec 25 2021, 12:36 AM

Remove comments (leave it to the commit message)

Harbormaster completed remote builds in B19623: Diff 46368.Dec 25 2021, 12:38 AM

Pratik Borhade (PratikPB2123) awarded a token.Dec 25 2021, 5:42 PM

This looks good, I just left one comment inline about the TODO comment.

source/blender/blenkernel/intern/object.cc
3901–3902	I feel like we should find an answer to which default is preferred before landing this patch, rather than adding a todo. Or the TODO could just be mentioned in the commit message, since it's not really a new inconsistency from this patch.

This revision is now accepted and ready to land.Dec 28 2021, 8:12 PM

Closed by commit rB1c7d7c915063: Fix T94113: Local view + Geometry Nodes is broken for instances (authored by Germano Cavalcante (mano-wii)). · Explain WhyDec 29 2021, 4:37 PM

This revision was automatically updated to reflect the committed changes.

Germano Cavalcante (mano-wii) marked an inline comment as done.

Germano Cavalcante (mano-wii) added a commit: rB1c7d7c915063: Fix T94113: Local view + Geometry Nodes is broken for instances.