Skip to content

Conversation

cdxntchou
Copy link

@cdxntchou cdxntchou commented Sep 25, 2021

Purpose of this PR

Backport to 2021.2 of:
Fix for: https://fogbugz.unity3d.com/f/cases/1367540/
Backport: https://fogbugz.unity3d.com/f/cases/1367746/

Backports:
2022.1: #5657
2021.2: #5801

Adding anisotropic option to the Sampler State node:
image

Docs ticket: https://jira.unity3d.com/browse/GSG-591


Testing status

Before: (no anisotropic)
image

After: (x16)
image

Added yamato test to test inline anisotropic sampler state node:
image

Yamato:

Nightly ShaderGraph on 2021.2: 🟢
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Fadd%252Fanisotropic/.yamato%252Fall-shadergraph.yml%2523Nightly_ShaderGraph_2021.2/8988874/job/pipeline

master: 34fcac4
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fstaging/.yamato%252Fall-shadergraph.yml%2523Nightly_ShaderGraph_2021.2/8988509/job/pipeline

  • Note that the OSX metal results on the anisotropic sampling test are incorrect, but it appears to just be from the integrated GPUs on the farm machines not supporting anisotropic. Running on a "real" OSX machine locally, the results look correct.

URP: 🟡 -- failures match master
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Fadd%252Fanisotropic/.yamato%252Fall-urp.yml%2523PR_URP_2021.2/9012128/job/pipeline

URP master:
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fstaging/.yamato%252Fall-urp.yml%2523PR_URP_2021.2/8982696/job/pipeline

failures: -- match master
URP_Lighting on Win_Vulkan_Standalone_mono_Linear on version 2021.2 -- failed same test : 202_SSAO_Depth 🟡

HDRP: 🟡 -- failures match master
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fsg%252Fadd%252Fanisotropic/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2/9012106/job/pipeline

HDRP master:
https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/2021.2%252Fstaging/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2/8982528/job/pipeline

failures: -- match master 🟡
HDRP on Linux_Vulkan_playmode_mono_Linear on version 2021.2 -- same 2 failed tests (after rerun) 🟡
HDRP on OSX_Metal_playmode_mono_Linear on version 2021.2 -- same 3 failed tests 🟡
HDRP on Win_Vulkan_playmode_mono_Linear on version 2021.2 -- same 2 failed tests 🟡
HDRP on Win_DX12_playmode_mono_Linear on version 2021.2 -- same 2 failed tests 🟡
HDRP on Win_DX11_playmode_XR_mono_Linear on version 2021.2 -- same 2 failed tests (after rerun) 🟡
HDRP on Win_DX11_playmode_mono_Linear on version 2021.2 -- same 2 failed tests 🟡

Console PR and Tests: 🟡
https://github.cds.internal.unity3d.com/unity/ScriptableRenderPipelinePrivate/pull/245

https://yamato.cds.internal.unity3d.com/jobs/339-ScriptableRenderPipelinePrivate/tree/2021.2%252Fstaging/.yamato%252Fall-shadergraph.yml%2523Nightly_ShaderGraph_2021.2/9040417/job/pipeline


Comments to reviewers

Notes for the reviewers you have assigned.

* Adding anisotropic option

* Shortening name

* Adding changelog

* Adding inline anisotropic test

* Updating Test Images

* Moved anisotropic control to node inspector, changed wording
# Conflicts:
#	com.unity.shadergraph/CHANGELOG.md
@github-actions
Copy link

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)

Shader Graph
/.yamato%252Fall-shadergraph.yml%2523PR_ShaderGraph_2021.2
Depending on your PR, you may also want
/.yamato%252Fall-shadergraph_builtin_foundation.yml%2523PR_ShaderGraph_BuiltIn_Foundation_2021.2
/.yamato%252Fall-shadergraph_builtin_lighting.yml%2523PR_ShaderGraph_BuiltIn_Lighting_2021.2

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.

@cdxntchou cdxntchou marked this pull request as ready for review September 25, 2021 19:05
@cdxntchou cdxntchou requested a review from a team as a code owner September 25, 2021 19:05
Copy link
Contributor

@jessebarker jessebarker left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@xiaoxicici xiaoxicici left a comment

Choose a reason for hiding this comment

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

Tested in both URP and HDRP, this new control working as expected.
Testing material set in URP:
image (24)
Testing materials in HDRP:
image

One comment I would like to mention is if having this option in the Node Settings only decreases the discoverability, comparing to the Filter and Wrap settings are configurable on the Sampler State node in the graph view:
image

@jessebarker jessebarker merged commit e9c368a into 2021.2/staging Sep 29, 2021
@jessebarker jessebarker deleted the 2021.2/sg/add/anisotropic branch September 29, 2021 16:38
@bencloward
Copy link
Contributor

"One comment I would like to mention is if having this option in the Node Settings only decreases the discoverability, comparing to the Filter and Wrap settings are configurable on the Sampler State node in the graph view"

Chris moved the option here after Alex requested that change.

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.

4 participants