Page MenuHome

Geometry Nodes: show "Show Texture in texture tab" button
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Feb 3 2021, 2:57 PM.

Details

Summary

This enables the quick access button [to show the relevant Texture in
the Properties Editor] for textures used in geometry nodes.

This goes in line to what we do for other textures:

  • modifier textures have this button
  • particle textures have this button
  • brush textures will soon have it, too (see D9813)

When outside of the Properties Editor, the button will always show, but will be inactive if no suiting Properties Editor to show the texture in can be found.

For this to work with geometry nodes, the following chages were done:

  • implement foreachTexLink for geonode modifiers
  • new buttons_texture_user_node_property_add() that stores prop as well as node
  • also use NODE_ACTIVE_TEXTURE flag in geometry nodetrees

notes:

  • this still uses the first suiting (as in: pinning does not interfere) Properties Editor it finds, this should (maybe?) find the closest Property Editor instead (see related feedback in D9813).

ref. T85278

Looks like this:

Diff Detail

Repository
rB Blender
Branch
T85278 (branched from master)
Build Status
Buildable 12612
Build 12612: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Feb 3 2021, 2:57 PM
Philipp Oeser (lichtwerk) created this revision.
Philipp Oeser (lichtwerk) retitled this revision from Geometry Nodes: show "Show Texture in texture tab" button to [WIP] Geometry Nodes: show "Show Texture in texture tab" button.Feb 3 2021, 2:58 PM

marking as WIP since the method to get the closest Properties Editor needs to be done still

regarding finding the closest Properties Editor I am a bit unsure what makes the most sense here:

  • source: the button position? node center? the region center?
  • Properties Editor: the Editor center? upper border? lower border?
  • or maybe just leave as is and take the first it finds?

(just trying not to overcomplicate things and to make it the most intuitive it can be...)

Leaving that up to discussion [next to the fact that the button vanishes when the correct tab is alread open], until then will remove [WIP] from the title to open up for feedback.

Philipp Oeser (lichtwerk) retitled this revision from [WIP] Geometry Nodes: show "Show Texture in texture tab" button to Geometry Nodes: show "Show Texture in texture tab" button.Feb 3 2021, 3:58 PM

regarding finding the closest Properties Editor I am a bit unsure what makes the most sense here:

  • source: the button position? node center? the region center?
  • Properties Editor: the Editor center? upper border? lower border?
  • or maybe just leave as is and take the first it finds?

(just trying not to overcomplicate things and to make it the most intuitive it can be...)

Leaving that up to discussion [next to the fact that the button vanishes when the correct tab is alread open], until then will remove [WIP] from the title to open up for feedback.

Maybe @Pablo Vazquez (pablovazquez) has an opinion?

This comment was removed by Philipp Oeser (lichtwerk).

@Philipp Oeser (lichtwerk) I think the button should always be there, and always enabled. Otherwise I think it is overkill to check this all at every draw call. If the cost was neglectable I would still have the button always there, but disabled when the editor+active texture was already correct

Philipp Oeser (lichtwerk) planned changes to this revision.Feb 3 2021, 5:47 PM

TODO:

  • always display button
  • button will select the right texture, but selecting a node out of many (if there are multiple) might select the wrong.
  • always have the button visible in nodes (Properties Editor itself hides it)
  • implement foreachTexLink for geonode modifiers
  • new buttons_texture_user_node_property_add() that stores prop as well as node
  • also use NODE_ACTIVE_TEXTURE flag in geometry nodetrees
Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

Hi @Philipp Oeser (lichtwerk) I believe there is some updates missing:

For the records for the second part of the video I use this file:

And I only get the button after I unpin one of the panels and merge into the other. (it happens offscreen).

Philipp Oeser (lichtwerk) planned changes to this revision.EditedFeb 4 2021, 7:13 PM
  • Does not work inside nested nodegroups
  • check missing updates
  • check issue with pinned Properties Editor

fix button not working inside nested nodegroups

Philipp Oeser (lichtwerk) planned changes to this revision.Feb 12 2021, 6:24 PM

these remain

  • check missing updates
  • check issue with pinned Properties Editor

fixed issue with pinned Properties Editor

improve button tooltip if disabled (also fixes prior memleak)

  • fix immediate update when unlinking a texture
  • hide button if no texture is assigned (we are still showing "New")

The functionality seems fine. I will leave the code review to someone else. (setting both Jacques and Hans as blocking, but as soon as one reviews it feel free to remove the other as blocking.

Also for anyone testing it remember to git rebase origin/master after arc patch. The current version this was patched on top of doesn't build for me due to a unrelated issue.

source/blender/editors/space_buttons/buttons_texture.c
393

Overall it looks good. It's not super pretty to keep these states in sync, but I did not expect this patch to be very pretty... It works well for me :)
Please fix the thing I mentioned, but then this does not have to go through a second round of review.

source/blender/editors/space_buttons/buttons_texture.c
188

Better just use RNA_Node and check that node->type == GEO_NODE_ATTRIBUTE_SAMPLE_TEXTURE before.

410

Comment style.

438

I'm not a huge fan of this, but it seems to make sense, and also Dalai approved the functionality.

This revision is now accepted and ready to land.Mar 2 2021, 10:54 AM