Fix #114657: Color attribute from applied GN modifier not active #114699

Closed

Philipp Oeser wants to merge 1 commits from lichtwerk/blender:114657 into main

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

Philipp Oeser commented

2023-11-10 12:18:02 +01:00

Member

Originally caused by 6514bb05ea.

One more cases where the active/default color attributes were not set
correctly was when a geometry nodes modifier was applied that was
storing named color attribute(s) [and that results in an color attribute
where previously none existed].

In this case, make the fist color attribute found active/default.

Similar to acfafeed88.

Originally caused by 6514bb05ea. One more cases where the active/default color attributes were not set correctly was when a geometry nodes modifier was applied that was storing named color attribute(s) [and that results in an color attribute where previously none existed]. In this case, make the fist color attribute found active/default. Similar to acfafeed88ee.

Philipp Oeser added 1 commit 2023-11-10 12:18:13 +01:00

a9bcf6763b Fix #114657 : Color attribute from applied GN modifier not active

Originally caused by 6514bb05ea.

One more cases where the active/default color attributes were not set
correctly was when a geometry nodes modifier was applied that was
storing named color attribute(s) [and that results in an color attribute
where previously none existed].

In this case, make the fist color attribute found active/default.

Similar to acfafeed88.

Philipp Oeser requested review from Hans Goudey 2023-11-10 12:18:22 +01:00

Philipp Oeser added this to the Modeling project 2023-11-10 12:18:33 +01:00

Philipp Oeser added the

label 2023-11-10 12:18:43 +01:00

Philipp Oeser commented

2023-11-10 12:28:06 +01:00

Author

Member

Will retarget the main branch in a bit (not a 4.0 regression, not for bcon4)...

Will retarget the `main` branch in a bit (not a 4.0 regression, not for bcon4)...

Philipp Oeser changed title from ~~Fix #114657: Color attribute from applied GN modifier not active~~ to Fix #114657: Color attribute from applied GN modifier not active

2023-11-10 14:03:05 +01:00

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

2023-11-10 14:03:10 +01:00

Hans Goudey commented

2023-11-15 11:18:16 +01:00

Member

We discussed this in the most recent nodes & physics meeting. We agreed that geometry nodes shouldn't really be concerned with the active status, since that's more UI-data. But that it's reasonable to set the default color attribute when it's created if there isn't already a color attribute. Not sure if that solves everything-- the topic is definitely more complicated than it seems at a glance.

We discussed this in the most recent nodes & physics meeting. We agreed that geometry nodes shouldn't really be concerned with the active status, since that's more UI-data. But that it's reasonable to set the default color attribute when it's created if there isn't already a color attribute. Not sure if that solves everything-- the topic is definitely more complicated than it seems at a glance.

Philipp Oeser commented

2023-11-15 11:31:12 +01:00

Author

Member

But that it's reasonable to set the default color attribute when it's created if there isn't already a color attribute.

That is all this PR does.

Not sure if that solves everything-- the topic is definitely more complicated than it seems at a glance.

Not sure if I understand correctly: handling active status in GN in any other way is out of scope of this PR (this is just for having something active after applying the modifier). Does this mean this PR has green light?

> But that it's reasonable to set the default color attribute when it's created if there isn't already a color attribute. That is all this PR does. > Not sure if that solves everything-- the topic is definitely more complicated than it seems at a glance. Not sure if I understand correctly: handling active status in GN in any other way is out of scope of this PR (this is just for having something active after applying the modifier). Does this mean this PR has green light?

Hans Goudey commented

2023-11-15 11:33:00 +01:00

Member

That is all this PR does.

This PR is setting the active color attribute, not just the default.

It's also setting the strings when applying the modifier. What we discussed was doing that in the store named attribute node. To me it seems nice if we try to keep applying modifiers from changing geometry as much as possible.

>That is all this PR does. This PR is setting the active color attribute, not just the default. It's also setting the strings when applying the modifier. What we discussed was doing that in the store named attribute node. To me it seems nice if we try to keep applying modifiers from changing geometry as much as possible.

Philipp Oeser commented

2023-11-15 12:02:29 +01:00

Author

Member

That is all this PR does.

This PR is setting the active color attribute, not just the default.

Ah, facepalm... OK, lets not mess with active, just default (could update the PR)

It's also setting the strings when applying the modifier. What we discussed was doing that in the store named attribute node. To me it seems nice if we try to keep applying modifiers from changing geometry as much as possible.

What does "also" mean here? Is there a way to set it other then via the strings?
Does this mean the store named attribute attribute node should get an option "Make default" if it stores a color? What should happen if this option is true for multiple of those nodes?

> >That is all this PR does. > > This PR is setting the active color attribute, not just the default. Ah, facepalm... OK, lets not mess with active, just default (could update the PR) > It's also setting the strings when applying the modifier. What we discussed was doing that in the store named attribute node. To me it seems nice if we try to keep applying modifiers from changing geometry as much as possible. What does "also" mean here? Is there a way to set it other then via the strings? Does this mean the `store named attribute` attribute node should get an option "Make default" if it stores a color? What should happen if this option is true for multiple of those nodes?

Hans Goudey commented

2023-11-15 14:04:30 +01:00

Member

What does "also" mean here? Is there a way to set it other then via the strings?

I guess "also" wasn't right. I mean it's just better to do it implicitly in the store named attribute node than it is to do it when applying the modifier. Ideally applying the modifier just moves the evaluated mesh to the original mesh, it shouldn't be a place for arbitrary operations IMO.

>What does "also" mean here? Is there a way to set it other then via the strings? I guess "also" wasn't right. I mean it's just better to do it implicitly in the store named attribute node than it is to do it when applying the modifier. Ideally applying the modifier just moves the evaluated mesh to the original mesh, it shouldn't be a place for arbitrary operations IMO.

Philipp Oeser commented

2023-11-15 14:27:53 +01:00

Author

Member

OK, time to step down I guess, will need some other solution then.

OK, time to step down I guess, will need some other solution then.

Philipp Oeser closed this pull request

2023-11-15 14:27:55 +01:00

Philipp Oeser referenced this pull request

2023-11-15 14:27:56 +01:00

Color attribute from applied Geometry Nodes modifier gets disabled render icon #114657

Pull request closed

Please reopen this pull request to perform a merge.

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

No description provided.