Skip to content

Conversation

arttu-peltonen
Copy link
Contributor

Purpose of this PR

Making sure Shadow Cascade debug colors in the viewport match the ones in the editor UI. The colors are not 100% the same in order to have better contrast in the viewport under varying lighting conditions.

URP before:
image

URP after:
image

HDRP before:
image

HDRP after:
image

Also fixed apparent typo resulting in too bright color in the UI:
before:
image
after:
image


Testing status

Checked URP & HDRP look as expected.

@arttu-peltonen arttu-peltonen requested a review from RSlysz August 25, 2021 08:48
@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)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2

URP
/.yamato%252Fall-urp.yml%2523PR_URP_2021.2

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_2021.2
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.

@arttu-peltonen arttu-peltonen marked this pull request as ready for review August 25, 2021 08:57
@arttu-peltonen arttu-peltonen requested review from a team as code owners August 25, 2021 08:57
Copy link
Contributor

@nigeljw-unity nigeljw-unity left a comment

Choose a reason for hiding this comment

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

LGTM

@sebastienlagarde sebastienlagarde removed the request for review from RSlysz August 25, 2021 11:55
@sebastienlagarde
Copy link
Contributor

good catch, thanks

Copy link
Contributor

@erikabar erikabar left a comment

Choose a reason for hiding this comment

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

Developer already tested this in URP and HDRP, approving without additional testing

@arttu-peltonen
Copy link
Contributor Author

arttu-peltonen commented Aug 26, 2021

Yamato against 21.2 is broken, so running PR jobs against latest green (cb3b74b20e592145e75fb03993f7a23e2c09e03a) now.

EDIT: rerunning jobs after Yamato is back up and HDRP test images are fixed:
HDRP https://unity-ci.cds.internal.unity3d.com/job/8394457
URP https://unity-ci.cds.internal.unity3d.com/job/8394595

@arttu-peltonen
Copy link
Contributor Author

@sebastienlagarde this should be good to go. Fixed HDRP test images for a test that used cascade debug mode, URP doesn't have a corresponding test. Failures in both test suites but couldn't spot anything that would seem related.

Copy link
Contributor

@RSlysz RSlysz left a comment

Choose a reason for hiding this comment

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

Nice catch for the typo. It should be me, sorry.

@sebastienlagarde sebastienlagarde merged commit 2116818 into master Aug 30, 2021
@sebastienlagarde sebastienlagarde deleted the srp/matching-shadow-cascade-debug-colors branch August 30, 2021 10:03
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