Skip to content

Conversation

Nightmask3
Copy link
Contributor

@Nightmask3 Nightmask3 commented May 18, 2020

Purpose of this PR

The Internal Inspector was introduced in this PR: #173. Since then I've continued to perform testing in order to find issues with the Inspector and verify the parts of the shader graph workflow that must now go through the inspector instead of the cog wheel menus and the blackboard rollouts. This PR contains fixes for a number of issues both reported by users/QA, and issues found by self-testing.

Capture

This PR includes fixes for:

  • Merged Alex's PR that was lost when replacing blackboard property editing with editing through the inspector: [Shader Graph] Fix Bug with Enum Keyword Reference Suffix #58
  • Window docking issues (resizing the graph window would not move the inspector window with it)
  • https://fogbugz.unity3d.com/f/cases/1244134/
  • Can't move scroll bar with mouse because resizing handle gets in the way for the right and bottom scroll bars
  • Remove precision from top toolbar as its in graph settings now, added button called "Graph Settings" which contains it instead (this is in preparation for the move to master stacks which expose many more graph level settings)
  • Fixed the issue where clicking anywhere in the inspector that’s not a control defocuses the current node and reverts the inspector contents to the graph’s view
  • Support for custom function and subgraph nodes
  • Remove cog wheel from all kind of nodes except master nodes
  • Undo selection does not update inspector
  • Fix for Custom function/Subgraph node when input is added/removed - reorderable list header no longer goes to "IList"
  • Subgraph node property drawers work correctly now

Testing status

Manual Tests

  • Ran the local ShaderGraph test project and verified all tests are passing
  • Closing and opening the graph to verify serialization of layout and window sizing/positions are preserved
  • Adding/removing/renaming/moving/deleting blackboard properties and keywords along with undo/redo at every stage, verified that no regressions here
  • Changing property/keyword values through the inspector and watching the graph preview update and the shaders recompile, everything seems to be working as expected
  • Selecting graph elements (like nodes and properties from the blackboards) and verifying that the inspector displays what is selected.
  • Selecting various kinds of elements and making sure that whenever they are deselected, the inspector is hidden when nothing remains to display, including when undo/redo changes active selection.
  • When color mode is set to "Precision" and the precision is changed through the graph settings in the inspector, verified that the entire graph colors update to Float/Half if the nodes are set to Inherit.

Comments to reviewers

  • This PR removes the cog wheel from all the different kinds of nodes except master nodes, which still retain the cog wheel menus. This is because the master nodes will go away soon (approximately June 1st) and it didn't make sense to do any work to port the master node settings over to the inspector as that code would be thrown away soon regardless.
  • The reason why this touches Universal and HDRP code is because of the removal of the IHasSettings interface (which doesn't need to exist in a world with the inspector and property drawers).
    If this is an issue I can add the code for them back in but the changes are fairly straightforward (i.e. the removal of the interface reference from all master nodes) and the functionality has been preserved. Eventually this code will be removed when master stacks lands regardless so the IHasSettings interface would need to be removed at some point.

sophiaaar and others added 17 commits May 15, 2020 14:24
* remove references to trunk or master

* update metafile
# Conflicts:
#	com.unity.shadergraph/Editor/Data/Graphs/GraphData.cs
# Conflicts:
#	com.unity.shadergraph/Editor/Data/Graphs/GraphData.cs
#	com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs
…anup

[skip ci]

# Conflicts:
#	com.unity.shadergraph/Editor/Drawing/Inspector/PropertyDrawers/ShaderGUIOverridePropertyDrawer.cs
#	com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs
#	com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs
…tor after deletion

# Conflicts:
#	com.unity.shadergraph/Editor/Drawing/Inspector/PropertyDrawers/ShaderInputPropertyDrawer.cs
# Conflicts:
#	com.unity.shadergraph/Editor/Data/Nodes/AbstractMaterialNode.cs
#	com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs
…ctor

# Conflicts:
#	com.unity.shadergraph/Editor/Drawing/Views/GraphEditorView.cs
#	com.unity.shadergraph/Editor/Drawing/Views/MaterialNodeView.cs
#	com.unity.shadergraph/Editor/Drawing/Views/PropertyNodeView.cs
com.unity.render-pipelines.core: 9.0.0-preview.29
com.unity.render-pipelines.high-definition-config: 9.0.0-preview.31
com.unity.render-pipelines.high-definition: 9.0.0-preview.29
com.unity.render-pipelines.lightweight: 9.0.0-preview.30
com.unity.render-pipelines.universal: 9.0.0-preview.30
com.unity.shadergraph: 9.0.0-preview.30
com.unity.visualeffectgraph: 9.0.0-preview.29
com.unity.render-pipelines.core: 9.0.0-preview.30
com.unity.render-pipelines.high-definition-config: 9.0.0-preview.32
com.unity.render-pipelines.high-definition: 9.0.0-preview.30
com.unity.render-pipelines.lightweight: 9.0.0-preview.31
com.unity.render-pipelines.universal: 9.0.0-preview.31
com.unity.shadergraph: 9.0.0-preview.31
com.unity.visualeffectgraph: 9.0.0-preview.30
* update dependency of promotion job

* add publish as a dependency
@Nightmask3 Nightmask3 requested review from a user and marctem May 18, 2020 19:05
@Nightmask3 Nightmask3 requested review from a team as code owners May 18, 2020 19:05
oleks-k and others added 3 commits May 19, 2020 09:41
* [9.x.x] Merge Hdrp/staging [Skip CI] (#544)

* Delete 4052_TAA.png.orig

* [10.x.x] Update threashold of HDRP DXR 802 deferred SSS test

* Update 801_SubSurfaceScatteringDeferred.unity

* update vulkan reference screenshots 4052 / 9301

* Update upm-ci-hdrp.yml
# Conflicts:
#	.yamato/upm-ci-abv.yml
#	.yamato/upm-ci-hdrp.yml
#	.yamato/upm-ci-hdrp_dxr.yml
#	.yamato/upm-ci-hdrp_hybrid.yml
#	.yamato/upm-ci-hdrp_standalone.yml
#	.yamato/upm-ci-packages.yml
#	.yamato/upm-ci-shadergraph.yml
#	.yamato/upm-ci-shadergraph_stereo.yml
#	.yamato/upm-ci-templates.yml
#	.yamato/upm-ci-universal.yml
#	.yamato/upm-ci-universal_hybrid.yml
#	.yamato/upm-ci-universal_stereo.yml
#	.yamato/upm-ci-vfx_lwrp.yml
#	.yamato/upm-ci-vfxmain.yml
#	.yamato/z_editor.yml
#	com.unity.render-pipelines.core/package.json
#	com.unity.render-pipelines.high-definition-config/package.json
#	com.unity.render-pipelines.high-definition/package.json
#	com.unity.render-pipelines.lightweight/package.json
#	com.unity.render-pipelines.universal/package.json
#	com.unity.shadergraph/package.json
#	com.unity.visualeffectgraph/package.json
@Nightmask3 Nightmask3 closed this May 19, 2020
@Nightmask3 Nightmask3 deleted the sg/inspector-bugfixes branch May 19, 2020 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants