GPv3: Bounding Box node #113790

Dalai Felinto · 2023-10-16T22:53:39+02:00

Dalai Felinto commented

2023-10-16 22:53:39 +02:00

Note that this node was already working. But it was creating a single
bounding box.

Similar to Convex Hull we are following the design defined in #113602
where each layer should create its own mesh instance.

Part of #113602.

Note that this node was already working. But it was creating a single bounding box. Similar to Convex Hull we are following the design defined in #113602 where each layer should create its own mesh instance. Part of #113602.

Dalai Felinto commented

2023-10-16 22:57:45 +02:00

Keep in mind that:

It is changing the GeometrySet::compute_boundbox_without_instances() function which is used elsewhere (e.g., BKE_object_boundbox_calc_from_evaluated_geometry).
Technically we could create a single Reference and add it multiple times. Wouldn't mind doing it, but only if needed.

My test file: greasepencil-nodes.blend

Keep in mind that: 1. It is changing the GeometrySet::compute_boundbox_without_instances() function which is used elsewhere (e.g., BKE_object_boundbox_calc_from_evaluated_geometry). 2. Technically we could create a single Reference and add it multiple times. Wouldn't mind doing it, but only if needed. My test file: [greasepencil-nodes.blend](/attachments/29450176-1c17-47e9-ac61-d065fb5bdc69)

greasepencil-nodes.blend

136 KiB

Dalai Felinto requested review from Hans Goudey 2023-10-16 22:57:56 +02:00

~~Dalai Felinto referenced this pull request 2023-10-16 22:58:39 +02:00~~

GPv3: Geometry Node support via the curve nodes #113602

Dalai Felinto force-pushed grease-nodes-bounding-box from 67017aab7d to d21997a2d7

2023-10-18 17:22:12 +02:00

Compare

Dalai Felinto requested review from Jacques Lucke 2023-10-18 17:32:03 +02:00

Jacques Lucke commented

2023-10-19 12:23:40 +02:00

Generally, outputting the individual bounding box as instances seems ok.

It feels like the Min and Max outputs of the node should still take the full grease pencil geometry into account though. Instances are not taken into account there, because that would be quite expensive in many cases. For grease pencil, it shouldn't really be more expensive than it is already.

I'll probably also change modify_geometry_sets sets today or tomorrow so that adding instances feels more safe.

Generally, outputting the individual bounding box as instances seems ok. It feels like the `Min` and `Max` outputs of the node should still take the full grease pencil geometry into account though. Instances are not taken into account there, because that would be quite expensive in many cases. For grease pencil, it shouldn't really be more expensive than it is already. I'll probably also change `modify_geometry_sets` sets today or tomorrow so that adding instances feels more safe.

Dalai Felinto force-pushed grease-nodes-bounding-box from d21997a2d7 to 6729fed3bc

2023-10-19 13:07:26 +02:00

Compare

Dalai Felinto commented

2023-10-19 13:08:27 +02:00

It feels like the Min and Max outputs of the node should still take the full grease pencil geometry into account though.

It makes sense, done! Test file: greasepencil-nodes.blend

> It feels like the Min and Max outputs of the node should still take the full grease pencil geometry into account though. It makes sense, done! Test file: [greasepencil-nodes.blend](/attachments/b9458ead-6619-47c8-92a2-078d0051db58)

greasepencil-nodes.blend

148 KiB

Dalai Felinto commented

2023-10-23 12:10:52 +02:00

I will abandon this patch now. While talking to @HooglyBoogly we thought it would be ok to keep the current behaviour we have in main (where the entire Grease Pencil object is considered for a single "Bounding Box".

This way this node to behave similar to what you get with from the "Dimensions" in the viewport.

Besides, this also matches the behaviour of connecting the "Bounding Box" node with the Object Info node - which is always takes the entire object into account.

I will abandon this patch now. While talking to @HooglyBoogly we thought it would be ok to keep the current behaviour we have in main (where the entire Grease Pencil object is considered for a single "Bounding Box". This way this node to behave similar to what you get with from the "Dimensions" in the viewport. Besides, this also matches the behaviour of connecting the "Bounding Box" node with the Object Info node - which is always takes the entire object into account.

Dalai Felinto closed this pull request

2023-10-23 12:10:57 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

GPv3: Bounding Box node #113790

Pull request closed