Skip to content

Conversation

JussiKnuuttila
Copy link
Contributor

@JussiKnuuttila JussiKnuuttila commented Sep 20, 2021

TODO:

  • Add DataExtraction pass to all URP text shaders (now only in Lit and Shader Graphs)
    ** Should URP also use ScenePickingPass to render object IDs, like HDRP? Would need to add ScenePickingPass everywhere, and the same is already necessary for DataExtraction (which is required to get world position and normals anyway, and is required by picking).
  • Add submesh ID extraction, so drag and drop can be fixed.
  • Selection outline code is still present here, but it will not be landed in the first PR. Probably it doesn't hurt to leave it in?
  • HDRP PickingSpaceTransforms.hlsl is commented out instead of deleted

Please read the Contributing guide before making a PR.

Checklist for PR maker

  • Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
  • Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?


Testing status

Describe what manual/automated tests were performed for this PR


Comments to reviewers

Notes for the reviewers you have assigned.

@github-actions
Copy link

github-actions bot commented Sep 20, 2021

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk
With changes to HDRP packages, you should also run
/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

URP
/.yamato%252Fall-urp.yml%2523PR_URP_trunk
With changes to URP packages, you should also run
/.yamato%252Fall-lightmapper.yml%2523PR_LightMapper_trunk

Shader Graph
/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_trunk
Depending on your PR, you may also want
/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_trunk
/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_trunk

SRP Core
You could run ABV on your branch before merging your PR, but it will start A LOT of jobs. Please be responsible about it and run it only when you feel the PR is ready:
/.yamato%252F_abv.yml%2523all_project_ci_trunk
Be aware that any modifications to the Core package impacts everyone in the Graphics repo so please discuss the PR with your lead.

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

# Conflicts:
#	com.unity.render-pipelines.universal/Editor/2D/ShaderGraph/Targets/UniversalSpriteCustomLitSubTarget.cs
#	com.unity.render-pipelines.universal/Editor/2D/ShaderGraph/Targets/UniversalSpriteLitSubTarget.cs
#	com.unity.render-pipelines.universal/Editor/2D/ShaderGraph/Targets/UniversalSpriteUnlitSubTarget.cs
#define RENDER_EMISSION_RGB 9
#define RENDER_WORLD_NORMALS_PIXEL_RGB 10
#define RENDER_SMOOTHNESS_R 11
#define RENDER_OCCLUSION_R 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should call this RENDER_AMBIENT_OCCLUSION as occlusion is undefined (in HDRP we have specular occlusion for example)

JussiKnuuttila and others added 11 commits November 25, 2021 13:51
# Conflicts:
#	com.unity.render-pipelines.high-definition/Runtime/Material/AxF/AxFProperties.hlsl
#	com.unity.render-pipelines.high-definition/Runtime/Material/Decal/DecalProperties.hlsl
#	com.unity.render-pipelines.high-definition/Runtime/Material/Lit/LitProperties.hlsl
#	com.unity.render-pipelines.high-definition/Runtime/Material/Unlit/UnlitProperties.hlsl
#	com.unity.render-pipelines.universal/Runtime/ScriptableRenderer.cs
#	com.unity.render-pipelines.universal/ShaderLibrary/UniversalDOTSInstancing.hlsl
# Conflicts:
#	com.unity.render-pipelines.core/ShaderLibrary/Packing.hlsl
#	com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalUnlitSubTarget.cs
#	com.unity.render-pipelines.universal/Runtime/Passes/DrawObjectsPass.cs
#	com.unity.render-pipelines.universal/Runtime/ScriptableRenderer.cs
…tributes to profiler wrappers to hide them in deep profiler (#6702)"

This reverts commit 8ad7300.
@sebastienlagarde
Copy link
Contributor

replicate on privarte gitub

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.

3 participants