Skip to content

Conversation

@adrien-de-tocqueville
Copy link
Contributor

@adrien-de-tocqueville adrien-de-tocqueville commented Jun 8, 2021

Purpose of this PR

Partial fix for https://fogbugz.unity3d.com/f/cases/1304189/

Fix outline rendering when TAA is enabled on the scene view (needs always refresh enabled)

The fix from aras for outline was apparently not working anymore because outline shader user the jittered matrix, and it caused flickering
before:
Flicker
after:
NoFlicker

This PR does not fix gizmo flickering though
This doesn't seem possible by using the same method as the one for the outline, because changing the depth offset for intersecting geometry just moves the intersection elsewhere, but does not remove it. For outline it does, because the issue is z-fighting


Testing status

Tested outline rendering for AxF, Unlit, Lit, LayeredLit, LitTessellation, LayeredLitTessellation, SG with/without tessellation

For tessellated shaders, we don't have access to the non jittered matrix frustum planes, so the outline tessellation is a bit different from the object tessellation in some case like this one, but it's minor i think
FlickerT

@adrien-de-tocqueville adrien-de-tocqueville requested review from a team and sebastienlagarde June 8, 2021 11:22
@github-actions github-actions bot added the HDRP label Jun 8, 2021
@github-actions
Copy link

github-actions bot commented Jun 8, 2021

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

Copy link
Contributor

@RemyUnity RemyUnity 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 merged commit 83ddbed into hd/bugfix Jun 10, 2021
@sebastienlagarde sebastienlagarde deleted the hd/fix-outline-flickering branch June 10, 2021 18:26
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