Fix error when using gizmos with camera stack in editor #5913
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. URP 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. |
|
It appears that you made a non-draft PR! |
|
|
phi-lira
approved these changes
Oct 11, 2021
phi-lira
pushed a commit
that referenced
this pull request
Oct 12, 2021
* Fix game window gizmos when camera stack is used * Remove Assert check for the last camera in stack * Add lastCameraInStack check
Closed
phi-lira
added a commit
that referenced
this pull request
Oct 18, 2021
commit e68a78e Merge: 6e8d51c d1a6ea2 Author: tomzig16 <tzigmantavicius@gmail.com> Date: Wed Oct 6 11:35:23 2021 +0300 Merge branch 'master' into urp/editor/fix-gizmos-with-camera-stack commit 6e8d51c Author: tomzig16 <tzigmantavicius@gmail.com> Date: Tue Oct 5 19:13:06 2021 +0300 Add lastCameraInStack check commit 055c1a0 Author: tomzig16 <tzigmantavicius@gmail.com> Date: Tue Oct 5 15:58:56 2021 +0300 Remove Assert check for the last camera in stack commit 1d0c961 Merge: 36372be 3fbbcd8 Author: tomzig16 <tzigmantavicius@gmail.com> Date: Tue Oct 5 15:47:30 2021 +0300 Merge branch 'master' into urp/editor/fix-gizmos-with-camera-stack commit 36372be Author: tomzig16 <tzigmantavicius@gmail.com> Date: Thu Sep 9 18:42:48 2021 +0300 Fix game window gizmos when camera stack is used
Merged
odbb
added a commit
that referenced
this pull request
Oct 19, 2021
* master: Enable iris normal for Eye shader (#5880) Update to HDRP Asset analytics (#6060) [HDPR] Update reference screenshots for Linux Vulkan Update 206_Motion_Vectors.png (#6046) Updated code owner for URP (#6052) [URP] Re-enable ios pbr tests #5983 Fix error when using gizmos with camera stack in editor #5913 emitting UI geometry for offscreen cameras too (case 1344969 fix) #5894 Remove deprecated UNITY_USE_NATIVE_HDR keyword from shader code. #5569 [XR][URP] Fix issue with vignette location in XR #5471 #5471
ellioman
added a commit
that referenced
this pull request
Jan 10, 2022
* Fix for specular color plus a first draft of a test scene for it * Minor color updates * Test updated * Adding Directional Light * Adding the new test scene to build settings * Reference Images * Redoing the specular bugfix after various changes happened on Master * Adding a Deferred Test Scene * Fixing Specular on Deferred and adding a test scene for it as well * Same fix but in ShaderGraph shaders * Filtering out the 011 deferred scene for Android Vulkan (Issue: 1340634) and updating three reference images * D3D11 updated reference image. * Set a valid stencil format when creating depthStencil render-target. * D3D12 reference image * D3D11 reference image * DX11 Deferred Reference image * Removing a scene from the testcase filters as the issue has been resolved * Changelog * [XR][URP] Fix issue with vignette location in XR (#5471) * [XR][URP] Fix issue with vignette location in XR Vignette center needs to respect the real view center since in XR, with asymmetric FOV, the center of the render taget is not always the center of the view. We deduce the real center via the projection matrix data and use that to offset the vignette's center. * Updated changelog * Extracted comon XRView code into ComputeEyeCenterUV & Allow more than 2 XR views * Fix vignette movement direction for XR when whe change the vignette "center" parameter * Returning to 2 views only for XR vignette. This will keep the shader as simple as possible, if we need to support more views in the future, we can restore this code * Use "Vector4.zero" instead of "new Vector4(0.0f, 0.0f, 0.0f, 0.0f)" * Only set v3 if we are using single pass That variable is only needed if single-pass instancing/multiview is active, so we can simplify the code to only do the asymetric FOV correction when we are in multi-pass mode. * More improvements - Renamed _Vignette_Params3 to _Vignette_ParamsXR - Moved all single-pass XR values in _Vignette_ParamsXR - Extracted code to correct eye center to XRPass class * Adding a comment to describe what ApplyXRViewCenterOffset does * Quick fix a comment that is not right anymore after moving the code * Remove deprecated UNITY_USE_NATIVE_HDR keyword from shader code. (#5569) * Fix error when using gizmos with camera stack in editor (#5913) * Fix game window gizmos when camera stack is used * Remove Assert check for the last camera in stack * Add lastCameraInStack check * Updating WindowsEditor DX11 reference image * Updating Android Vulkan & iOS Metal reference images. Upping the threshold a tiny bit. * Trying a new reference image for WinPlayer DX11 & DX12 * Disabling XR capability for the new test as it breaks due to the Development Build label. * Updating reference images for VFX due to the changes done with my fix. * Changelog * Updating materials * Updating reference images for VFX OSX Editor on Metal Co-authored-by: Andrem <andrem@unity3d.com> Co-authored-by: Kay Chang <kaychang@unity3d.com> Co-authored-by: Marc-Andre Loyer <82059196+maloyer-unity@users.noreply.github.com> Co-authored-by: Pema Malling <pema99@users.noreply.github.com> Co-authored-by: Tomas Zigmantavičius <30701728+tomzig16@users.noreply.github.com> Co-authored-by: Felipe Lira <felipedrl@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
This PR fixes: https://fogbugz.unity3d.com/f/cases/1347472/
Previously there was an assert that was used as camera stack was not implemented, but now it works with it. When PR was made to change the if statement, assertion was forgotten hence it causes the regression. After removing it everything works correctly.
Testing status
Tested locally + automated ABV tests
Comments to reviewers
Although bug report tells that it is not a regression, we found that the issue appeared indirectly after the changes made in #4034
I will check for a need of backports as this issue was introduced through a different change.