Urp/fix to add tooltip of ss-shadows #5034

Seongdae · 2021-07-02T09:43:47Z

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

Fix case 1327356, https://fogbugz.unity3d.com/f/cases/1327356/
And While I was trying large-scale shadows using a RendererFeature of screen space shadows, those shadows were faded out from cascade distance. So I request my idea of shadow-fading on only realtime shadows in screen space shadows pass. Here is what I tried, https://idea-registry.cloud.unity3d.com/groups/hackweek-2021/ideas/1316

Testing status

Describe what manual/automated tests were performed for this PR

Comments to reviewers

Notes for the reviewers you have assigned.

…pass

github-actions · 2021-07-07T07:40:44Z

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)

URP
/.yamato%252Fall-urp.yml%2523PR_URP_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.

com.unity.render-pipelines.universal/Documentation~/renderer-feature-screen-space-shadows.md

Seongdae · 2021-07-08T06:18:15Z

com.unity.render-pipelines.universal/ShaderLibrary/Shadows.hlsl

 #endif

+#if defined(LIGHTMAP_SHADOW_MIXING) || defined(SHADOWS_SHADOWMASK) || !defined(_MAIN_LIGHT_SHADOWS_SCREEN)
    return MixRealtimeAndBakedShadows(realtimeShadow, bakedShadow, shadowFade);


@phi-lira the main goal of this PR is to fix a fugbuz.

And,
While I was working on far-distance shadows using my another screen space shadows renderer feature, the cascade distance faded out those shadows although there are far distance shadows in the scene.
I thought it needs to exclude fade-out in getting shadows from screen space shadow texture when it didn't have baked shadows. This is my opinion and if this idea had problems in the current situation. let me revert it to the original codes. and I'll request another PR for it.

@phi-lira plz check this PR and tell me your opinions

@lukaschod can you have a look at this?

I am not sure I undestand fully the intention behind the macro usage here. @Seongdae could you eleborate a bit more, as seems like u are removing shadow fade in case of no mixed lighting or am I wrong?

Move shadow fade to screen space shadows pass when screen space shadows' feature was enabled in case of no mixed lighting.

In this hackweek, I tried to make a custom screen space shadows' feature for shadowing large-scale, and it could cast shadows on anywhere, but it was faded out beyond the distance urp set. At that moment I though any user can also have trouble with it by their own shadowing solution.

k I understand I would prefer to avoid adding screen space logic into this function. As we already have user pain where we have defines that change completetly functionality without users aware of it.
Maybe we can simply have ScreenSpaceShadows.shader instead of calling MainLightRealtimeShadow use MainLightRealtimeShadow in case of no mixed lighting?

@lukaschod let me think about it and request another PR. the code will be restored in this PR.

phi-lira

Please add graphics tests. Can be in follow up PR or in this one.

phi-lira · 2021-09-10T08:04:24Z

com.unity.render-pipelines.universal/ShaderLibrary/Shadows.hlsl

 #endif

+#if defined(LIGHTMAP_SHADOW_MIXING) || defined(SHADOWS_SHADOWMASK) || !defined(_MAIN_LIGHT_SHADOWS_SCREEN)
    return MixRealtimeAndBakedShadows(realtimeShadow, bakedShadow, shadowFade);


@lukaschod can you have a look at this?

JordanL8 · 2021-09-30T09:17:50Z

Hey @Seongdae, I'll take a look at the docs for this today 🙂

Seongdae · 2021-09-30T09:18:55Z

Hey @JordanL8 thanks :)

Seongdae and others added 6 commits June 29, 2021 10:01

Add a editor class of screen space shadows

a681128

Merge branch 'master' into urp/fixToHideSettingsOfSSShadows

0faa9ca

Missing meta file

c9f614f

Apply shadow-fading to a output of screen space shadows

473c72d

Update CHANGELOG.md

e38368f

Change shadow-fading on only realtime shadow in screen space shadows …

b5b3e96

…pass

Seongdae added the universal label Jul 2, 2021

Seongdae requested a review from phi-lira July 2, 2021 09:43

Seongdae self-assigned this Jul 2, 2021

Seongdae added 2 commits July 2, 2021 18:47

Fix to fade out main light shadows not using screen space shadows

4dfe365

Add tooltip for screen space shadows

30b00a9

Seongdae changed the title ~~Urp/fix to hide settings of ss shadows~~ Urp/fix to add tooltip of ss-shadows Jul 7, 2021

Seongdae added 11 commits July 7, 2021 19:12

Update CHANGELOG.md

9625ad4

Add images for a documentation of screen space shadows

20a1490

Add a documentation of screen space shadows

52f13c4

Update a docs of screen space shadows

31d8ec5

Update renderer-feature-screen-space-shadows.md

4a2716b

Update renderer-feature-screen-space-shadows.md

9cca495

Update renderer-feature-screen-space-shadows.md

03679cf

Update renderer-feature-screen-space-shadows.md

c5c9a4e

Update renderer-feature-screen-space-shadows.md

9f03efb

Update renderer-feature-screen-space-shadows.md

019f683

Update renderer-feature-screen-space-shadows.md

1598191

Seongdae requested a review from oleks-k July 8, 2021 05:54

Seongdae commented Jul 8, 2021

View reviewed changes

com.unity.render-pipelines.universal/Documentation~/renderer-feature-screen-space-shadows.md Show resolved Hide resolved

Seongdae commented Jul 8, 2021

View reviewed changes

Seongdae added 2 commits August 26, 2021 15:50

Merge branch 'master' into urp/fixToHideSettingsOfSSShadows

97f90e6

Update renderer-feature-screen-space-shadows.md

f2dae6d

phi-lira approved these changes Sep 10, 2021

View reviewed changes

phi-lira requested a review from lukaschod September 10, 2021 08:10

JordanL8 requested review from JordanL8 and removed request for oleks-k September 30, 2021 09:17

Reviewed the documentation and removed now unused images

c437d56

JordanL8 approved these changes Oct 1, 2021

View reviewed changes

Seongdae added 4 commits October 5, 2021 11:27

Update images of ssshadows

24495bb

Revert to the origin

451068d

Merge branch 'master' into urp/fixToHideSettingsOfSSShadows

2405d01

Merge branch 'master' into urp/fixToHideSettingsOfSSShadows

3d323f4

lukaschod approved these changes Oct 6, 2021

View reviewed changes

Seongdae added the ready-to-merge Add this tag whenever your PR is ready to be merged. i.e, non draft, all reviewers approved, ABV label Oct 6, 2021

Seongdae marked this pull request as ready for review October 6, 2021 07:28

Seongdae requested review from a team as code owners October 6, 2021 07:28

phi-lira approved these changes Oct 6, 2021

View reviewed changes

phi-lira changed the base branch from master to universal/staging October 6, 2021 14:27

phi-lira merged commit 6ee599d into universal/staging Oct 6, 2021

phi-lira deleted the urp/fixToHideSettingsOfSSShadows branch October 6, 2021 14:28

phi-lira mentioned this pull request Oct 6, 2021

Universal Staging #5943

Merged

JordanL8 mentioned this pull request Nov 5, 2021

Added the Screen Space Shadows Renderer Feature doc to the table of contents #6218

Merged

Urp/fix to add tooltip of ss-shadows #5034

Urp/fix to add tooltip of ss-shadows #5034

Uh oh!

Conversation

Seongdae commented Jul 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for PR maker

Purpose of this PR

Testing status

Comments to reviewers

Uh oh!

github-actions bot commented Jul 7, 2021

Uh oh!

Uh oh!

Seongdae Jul 8, 2021

Choose a reason for hiding this comment

Uh oh!

Seongdae Aug 26, 2021

Choose a reason for hiding this comment

Uh oh!

phi-lira Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

lukaschod Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Seongdae Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

lukaschod Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

Seongdae Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

phi-lira left a comment

Choose a reason for hiding this comment

Uh oh!

phi-lira Sep 10, 2021

Choose a reason for hiding this comment

Uh oh!

JordanL8 commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Seongdae commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Seongdae commented Jul 2, 2021 •

edited

Loading

lukaschod Sep 20, 2021 •

edited

Loading

JordanL8 commented Sep 30, 2021 •

edited

Loading