Fix: Geometry Nodes: Create instance attribute of the right type #110146

Merged

Hans Goudey merged 8 commits from mod_moder/blender:tmp_fix_instances_crash into blender-v4.0-release

2023-10-14 10:54:41 +02:00

Iliya Katushenock commented

2023-07-15 21:40:55 +02:00

Member

attributes_to_propagate is map of all attributes to propagate from different
components. For attribute propagation three different components is taken.
Each one can have named attribute with specific type. Currenct implementation
of propagation: Try to make implicit share attribute. Or create/copy.
This dosn't takes into account attributes_to_propagate' data type info.
So, this cause crash for case, there multiple component have same name attribute
with different type.
Fix: Try to make implicit share attribute with correct type. Or create, copy typed version.

`attributes_to_propagate` is map of all attributes to propagate from different components. For attribute propagation three different components is taken. Each one can have named attribute with specific type. Currenct implementation of propagation: Try to make implicit share attribute. Or create/copy. This dosn't takes into account `attributes_to_propagate`' data type info. So, this cause crash for case, there multiple component have same name attribute with different type. Fix: Try to make implicit share attribute with correct type. Or create, copy typed version.

Iliya Katushenock added the

label 2023-07-15 21:40:55 +02:00

Iliya Katushenock added 1 commit 2023-07-15 21:41:06 +02:00

12bee7ac73 init

Iliya Katushenock added this to the Nodes & Physics project 2023-07-15 21:41:10 +02:00

Iliya Katushenock requested review from Hans Goudey 2023-07-15 21:41:14 +02:00

Hans Goudey requested changes 2023-07-24 21:40:03 +02:00

Hans Goudey left a comment

Member

Hmm. Thanks for the fix. The existing src.varray.size() == dst_component.instances_num() check is supposed to account for this, but it misses that the result instances are resized every time.

I wonder if we should handle this differently actually, and just create bke::Instances for each component, then join them afterwards. I'm not sure if it's worth optimizing for the case where points are used from multiple source components. Best to check Jacques' opinion about that.

Hmm. Thanks for the fix. The existing `src.varray.size() == dst_component.instances_num()` check is supposed to account for this, but it misses that the result instances are resized every time. I wonder if we should handle this differently actually, and just create `bke::Instances` for each component, then join them afterwards. I'm not sure if it's worth optimizing for the case where points are used from multiple source components. Best to check Jacques' opinion about that.

source/blender/nodes/geometry/nodes/node_geo_instance_on_points.cc Outdated

						
				@ -161,3 +162,4 @@

				  for (const auto item : attributes_to_propagate.items()) {

				    const AttributeIDRef &id = item.key;

				    const eCustomDataType data_type = item.value.data_type;

				    const bke::GAttributeReader src = src_attributes.lookup(id, ATTR_DOMAIN_POINT);

Hans Goudey commented

2023-07-24 21:40:00 +02:00

Member

I think this achieves the same thing, but more automatically:

    const bke::GAttributeReader src = src_attributes.lookup(
        id, ATTR_DOMAIN_POINT, item.value.data_type);

I think this achieves the same thing, but more automatically: ``` const bke::GAttributeReader src = src_attributes.lookup( id, ATTR_DOMAIN_POINT, item.value.data_type); ```

mod_moder marked this conversation as resolved

Iliya Katushenock changed title from ~~Fix: Geometry Nodes: Create instance attribute of the right type~~ to Fix: Geometry Nodes: Create instance attribute of the right type

2023-10-13 19:52:25 +02:00

mod_moder changed target branch from main to blender-v4.0-release

2023-10-13 19:52:59 +02:00

Iliya Katushenock commented

2023-10-13 19:53:50 +02:00

Author

Member

Hmm. Thanks for the fix. The existing src.varray.size() == dst_component.instances_num() check is supposed to account for this, but it misses that the result instances are resized every time.

I doubt it's related. The problem is the difference in the type of data in geometries when they are joined to instances domain.

> Hmm. Thanks for the fix. The existing src.varray.size() == dst_component.instances_num() check is supposed to account for this, but it misses that the result instances are resized every time. I doubt it's related. The problem is the difference in the type of data in geometries when they are joined to instances domain.

Iliya Katushenock added 3 commits 2023-10-13 19:59:20 +02:00

fbb0278823 init

437c39c51c progress

e5e30a99f4 fix merge conflict

Iliya Katushenock added 1 commit 2023-10-13 20:00:07 +02:00

9795d0913c Merge branch 'blender-v4.0-release' into tmp_fix_instances_crash

Iliya Katushenock requested review from Hans Goudey 2023-10-13 20:00:17 +02:00

Iliya Katushenock added 2 commits 2023-10-13 20:04:42 +02:00

8b4b61584a cleanup

97d819bd04 Merge branch 'tmp_fix_instances_crash' of projects.blender.org:mod_moder/blender into tmp_fix_instances_crash

Iliya Katushenock added this to the 4.0 milestone 2023-10-13 20:43:19 +02:00

Iliya Katushenock added 1 commit 2023-10-14 09:50:44 +02:00

buildbot/vexp-code-patch-coordinator Build done. Details

b4225aa005 progress

Hans Goudey commented

2023-10-14 10:14:31 +02:00

Member

@blender-bot build

@blender-bot build

Hans Goudey approved these changes 2023-10-14 10:15:49 +02:00

Hans Goudey left a comment

Member

Looks good to me, thanks. I'll commit this pending test results.

Looks good to me, thanks. I'll commit this pending test results.

Iliya Katushenock commented

2023-10-14 10:49:01 +02:00

Author

Member

The only one failed test is bf_asset_system_tests, currently this is failure on all build bots.

The only one failed test is `bf_asset_system_tests`, currently this is failure on all build bots.

Hans Goudey merged commit 690deeda18 into blender-v4.0-release

2023-10-14 10:54:41 +02:00

Hans Goudey referenced this issue from a commit

2023-10-14 10:54:42 +02:00

Fix: Geometry Nodes: Create instance attribute of the right type

Iliya Katushenock deleted branch tmp_fix_instances_crash

2023-10-14 11:00:26 +02:00

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#110146

No description provided.