Skip to content

Enable "Shaded Wireframe" scene view mode in HDRP & URP [Need C++ PR not backport on 2020.1 #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2020

Conversation

aras-p
Copy link
Contributor

@aras-p aras-p commented May 4, 2020

Purpose of this PR

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

Depends on C++ PR here: https://ono.unity3d.com/unity/unity/pull-request/105681/_/graphics/srp/textured-wire


Testing status

Manual Tests: What did you do?

  • Opened test project + Run graphic tests locally
  • Built a player
  • Checked new UI names with UX convention
  • Tested UI multi-edition + Undo/Redo + Prefab overrides + Alignment in Preset
  • C# and shader warnings (supress shader cache to see them)
  • Checked new resources path for the reloader (in developer mode, you have a button at end of resources that check the paths)
  • Other:

Automated Tests: What did you setup? (Add a screenshot or the reference image of the test please)

Yamato: (Select your branch):
https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics

Any test projects to go with this to help reviewers?


Comments to reviewers

Notes for the reviewers you have assigned.

Will need graphics/srp/textured-wire codebase branch to work
@aras-p aras-p requested a review from a team as a code owner May 4, 2020 12:35
@aras-p aras-p marked this pull request as draft May 4, 2020 12:43
This way it's not coupled to Gizmos rendering or not
@phi-lira
Copy link
Contributor

phi-lira commented May 6, 2020

We should we bump the unityRelease in package.json to match the version of Unity that contains this new API.

@sebastienlagarde sebastienlagarde changed the title Enable "Shaded Wireframe" scene view mode in HDRP & URP Enable "Shaded Wireframe" scene view mode in HDRP & URP [depends on C++] May 8, 2020
@sebastienlagarde sebastienlagarde marked this pull request as ready for review May 22, 2020 17:36
@sebastienlagarde
Copy link
Contributor

C++ PR have landed now, let's merge it (waiting for Yamato :) )

@sebastienlagarde sebastienlagarde changed the title Enable "Shaded Wireframe" scene view mode in HDRP & URP [depends on C++] Enable "Shaded Wireframe" scene view mode in HDRP & URP May 22, 2020
@sebastienlagarde
Copy link
Contributor

Yamato is green and the PR have landed. Merging

@sebastienlagarde sebastienlagarde merged commit 7617961 into master May 22, 2020
@sebastienlagarde sebastienlagarde deleted the scene/enable-textured-wire branch May 22, 2020 22:53
sebastienlagarde added a commit that referenced this pull request May 23, 2020
* Enable "Shaded Wireframe" scene view mode in HDRP & URP

Will need graphics/srp/textured-wire codebase branch to work

* Use new DrawWireOverlay API point to render wireframe

This way it's not coupled to Gizmos rendering or not

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>
@aras-p
Copy link
Contributor Author

aras-p commented May 25, 2020

@sebastienlagarde @phi-lira: since this now requires a new API that is only since 2020.2, does that mean the usages should get wrapped into some sort of #if check? Or are master branches of HDRP & URP only compatible with 2020.2 anyway at this point?

@sebastienlagarde
Copy link
Contributor

sebastienlagarde commented May 25, 2020

Do we plan a backport of it? (if possible would be nice)
If no backport is plan, then we don't care, the code will only be in 10.x package and only for 2020.2.

Otherwise, we can backport this PR in 9.x and 8.x when the c++ backport land on 2020.1

@phi-lira
Copy link
Contributor

@aras-p It would also be nice to have a wrap with unity version macro.

"If no backport is plan, then we don't care, the code will only be in 10.x package and only for 2020.2." a good point of wrapping in macros for 2020.2 f.ex, is that when bisecting an issue we can run older package with recent version of Unity. This helps bisecting a lot when we suspect the regression is in Unity side.

@sebastienlagarde
Copy link
Contributor

sebastienlagarde commented May 25, 2020

a good point of wrapping in macros for 2020.2 f.ex, is that when bisecting an issue we can run older
This doesn't help you here

if you have #ifdef UNITY_VERSION_2020_2_OR_NEWER, it is define since we have branch off 2020.2.
So the guard is useless for a C++ bisecting on commit prior to the change of shaded wireframe.

I prefer to avoid to pollute code with unecesary #ifdef

is that when bisecting an issue we can run older package with recent version of Unity.

Older package will not have this code, so it will work correctly.
Here I suppose that you talk about having the C++ code with guardband right? Which should have been part of the c++ PR in this case.

Unless I missed a case?

@sebastienlagarde sebastienlagarde changed the title Enable "Shaded Wireframe" scene view mode in HDRP & URP Enable "Shaded Wireframe" scene view mode in HDRP & URP [Need C++ PR not backport on 2020.1 Jul 7, 2020
sebastienlagarde added a commit that referenced this pull request Nov 16, 2020
* Enable "Shaded Wireframe" scene view mode in HDRP & URP

Will need graphics/srp/textured-wire codebase branch to work

* Use new DrawWireOverlay API point to render wireframe

This way it's not coupled to Gizmos rendering or not

Co-authored-by: Sebastien Lagarde <sebastien@unity3d.com>
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