Fix: Geometry Nodes: Sample curve crash with invalid curve indices #110099

Open

Iliya Katushenock wants to merge 3 commits from mod_moder/blender:tmp_fix_sample_curves_crash into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

Iliya Katushenock commented

2023-07-14 12:48:47 +02:00

Member

VectorSet used to sample index of real deduplicated groups index of curves.
VectorSet filled by only valid indices of curves (if user input cantains in curves range).
But to sample this, only valid indices should be used now.
Fix: create mask_valids, mask_unvalids for curve indices and use it to
fill invalids by zero and sample all valids.

VectorSet used to sample index of real deduplicated groups index of curves. VectorSet filled by only valid indices of curves (if user input cantains in curves range). But to sample this, only valid indices should be used now. Fix: create `mask_valids`, `mask_unvalids` for curve indices and use it to fill invalids by zero and sample all valids.

Iliya Katushenock added the

label 2023-07-14 12:48:47 +02:00

Iliya Katushenock added 1 commit 2023-07-14 12:48:58 +02:00

cbb3cef4ab init

Iliya Katushenock added this to the Nodes & Physics project 2023-07-14 12:49:16 +02:00

Iliya Katushenock requested review from Hans Goudey 2023-07-14 12:49:27 +02:00

Hans Goudey reviewed 2023-07-14 13:38:45 +02:00

Hans Goudey left a comment

Member

Thanks!

Thanks!

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

						
				@ -416,1 +405,3 @@

				        });

				    IndexMaskMemory memory;

				    IndexMask mask_valids;

Hans Goudey commented

2023-07-14 13:38:29 +02:00

Member

Small naming thing--

mask_valids -> mask_valid
mask_invalids -> mask_invalid

Small naming thing-- - `mask_valids` -> `mask_valid` - `mask_invalids` -> `mask_invalid`

mod_moder marked this conversation as resolved

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

						
				@ -417,1 +413,4 @@

				        const int curve_index = curve_indices[index];

				        return curves_range.contains(curve_index);

				      });

				      mask_invalids = IndexMask::from_predicate(

Hans Goudey commented

2023-07-14 13:38:30 +02:00

Member

Maybe it's better to build this with IndexMask::complement(), so it can be skipped if all the indices were valid

Maybe it's better to build this with `IndexMask::complement()`, so it can be skipped if all the indices were valid

Iliya Katushenock commented

2023-07-14 13:43:09 +02:00

Author

Member

Yeah, i was try, but complement use range as univers instead of mask. So it can't be used in this case.

Yeah, i was try, but `complement` use range as univers instead of mask. So it can't be used in this case.

mod_moder marked this conversation as resolved

Iliya Katushenock added 1 commit 2023-07-14 13:47:39 +02:00

26ef0b7ff0 progress

Iliya Katushenock changed title from ~~Fix: Crash Smaple Curve node~~ to Fix: Geometry Nodes: Crash Smaple Curve node for incorrect curve index field

2023-07-15 21:49:14 +02:00

Hans Goudey changed title from ~~Fix: Geometry Nodes: Crash Smaple Curve node for incorrect curve index field~~ to Fix: Geometry Nodes: Crash Smaple Curve node with invalid curve indices

2023-07-16 05:12:48 +02:00

Hans Goudey changed title from ~~Fix: Geometry Nodes: Crash Smaple Curve node with invalid curve indices~~ to Fix: Geometry Nodes: Sample curve crash with invalid curve indices

2023-07-16 05:13:07 +02:00

Hans Goudey commented

2023-07-16 05:14:58 +02:00

Member

Before moving forward with this fix, I'd like to look into dealing with the invalid indices at a different level, so the node itself doesn't have to deal with this. Some nodes preprocess input fields to do that, with a separate operation. But I don't have a specific idea yet.

Before moving forward with this fix, I'd like to look into dealing with the invalid indices at a different level, so the node itself doesn't have to deal with this. Some nodes preprocess input fields to do that, with a separate operation. But I don't have a specific idea yet.

Iliya Katushenock commented

2023-08-17 03:00:45 +02:00

Author

Member

I made this solution as a test: #111197

I made this solution as a test: https://projects.blender.org/blender/blender/pulls/111197

Iliya Katushenock referenced this pull request

2023-09-23 15:25:43 +02:00

Fix: Hang on invalid index in Sample Curve Node #112775

Iliya Katushenock added 1 commit 2023-09-23 15:53:50 +02:00

41a018dc66 Merge branch 'main' into tmp_fix_sample_curves_crash

This pull request has changes conflicting with the target branch.

source/blender/nodes/geometry/nodes/node_geo_curve_sample.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u tmp_fix_sample_curves_crash:mod_moder-tmp_fix_sample_curves_crash

git checkout mod_moder-tmp_fix_sample_curves_crash

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#110099

No description provided.