Skip to content

Conversation

@thomas-zeng
Copy link
Contributor

@thomas-zeng thomas-zeng commented Oct 20, 2020

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


Testing status

Describe what manual/automated tests were performed for this PR


Comments to reviewers

  • Fixed xrRendering is true when no XR display is available.
  • If no XR display provider is installed, when creating the new camera, the camera target eye will now set to None by default.

…alled.

Added runtime XR on/off check for XR camera.
@thomas-zeng thomas-zeng marked this pull request as ready for review October 20, 2020 23:11
@thomas-zeng thomas-zeng requested a review from a team as a code owner October 20, 2020 23:11
#if ENABLE_VR && ENABLE_XR_MODULE
List<XRDisplaySubsystemDescriptor> displayDescriptors = new List<XRDisplaySubsystemDescriptor>();
SubsystemManager.GetSubsystemDescriptors(displayDescriptors);
additionData.allowXRRendering = displayDescriptors.Count > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be changing serialized component data in OnEnable. We shouldn't have to change this value based on the presence of XR. It should be enough to have the checks during rendering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the fogbugz case, the runtime check is good enough.
The code here improved the default value for camera's target eye. It will defaults new camera's target eye field to None when it gets created in editor and no XR provider is installed.

I will revert the change here and put it into another PR. We can review it separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in
1d87eaa

@erikabar erikabar self-requested a review October 26, 2020 14:05
@phi-lira phi-lira changed the title [10.x.x] Fixed per camera MSAA regression [11.x.x] Fixed per camera MSAA regression Oct 27, 2020
@phi-lira phi-lira changed the base branch from master to universal/staging-v3 October 27, 2020 15:38
@phi-lira
Copy link
Contributor

merging to stage branch

@phi-lira phi-lira merged commit 57b391c into universal/staging-v3 Oct 27, 2020
@phi-lira phi-lira deleted the universal/xr/fix-camera-msaa-regression branch October 27, 2020 15:38
phi-lira added a commit that referenced this pull request Oct 29, 2020
* [11.x.x] Fixing NullReferenceException with Lift Gamma Gain & upgrading URP Package (#2386)

* Fixing issue 1283588

* Similar fix to ShadowsMidtonesHighlightsEditor.cs

* Minor fixes

* More minor fixes

* [11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 (#1922)

* Bugfix 1162556

* Fixing issue 1226288

* Minor fix to ParticlesEditorPass

* Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth

* Fixing compile errors

* Fixes based on review

* Fixing an copy/paste error

* Fixing things based on reviews

* Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS

* Changing the message from info to warning

* Changelog

* changelog

* Bugfix: The code was overriding the depth texture for overlay cameras, which is incorrect.

* [11.x.x] MSAA fix for Metal MacOS/Editor (#2391)

* Metal MacOS/Editor MSAA fix. Require an explicit MSAA resolve pass unless the platform is mobile

* Added changelog entry

* [11.x.x] Fixed per camera MSAA regression  (#2293)

* Default to XR off when creating the camera and no XR provider is installed.
Added runtime XR on/off check for XR camera.

* Updated CHANGELOG.md and small format fixes.

* Reverted changes in URPCameraEditor and URPCameraData.

* [11.x.x] Early camera color target assignment (#2377)

* Assign the camera color target early in case it is needed during AddRenderPasses

* Update to reflect PR feedback

* Get rid of warnings from accessing cameraColorTarget during AddRenderPasses

* Update CHANGELOG.md

* Removed tile shaders are currently unused. (#2383)

Co-authored-by: Elvar Örn Unnþórsson <[email protected]>
Co-authored-by: manuele-bonanno <[email protected]>
Co-authored-by: thomas-zeng <[email protected]>
Co-authored-by: Peter Bay Bastian <[email protected]>
Co-authored-by: Kay Chang <[email protected]>
phi-lira pushed a commit that referenced this pull request Oct 29, 2020
* Default to XR off when creating the camera and no XR provider is installed.
Added runtime XR on/off check for XR camera.

* Updated CHANGELOG.md and small format fixes.

* Reverted changes in URPCameraEditor and URPCameraData.
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md
phi-lira added a commit that referenced this pull request Oct 29, 2020
* [11.x.x] Fixing NullReferenceException with Lift Gamma Gain & upgrading URP Package (#2386)

* Fixing issue 1283588

* Similar fix to ShadowsMidtonesHighlightsEditor.cs

* Minor fixes

* More minor fixes
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md

* [11.x.x] Soft Particle bugfixes. Cases 1162556, 1256953& 1226288 + 1268079 (#1922)

* Bugfix 1162556

* Fixing issue 1226288

* Minor fix to ParticlesEditorPass

* Removing HasDepthTexture() from the shaders and instead setting the depth texture to black/white if we're not doing a prepass or copydepth

* Fixing compile errors

* Fixes based on review

* Fixing an copy/paste error

* Fixing things based on reviews

* Renaming PARTICLES_EDITOR define to PARTICLES_EDITOR_META_PASS

* Changing the message from info to warning

* Changelog

* changelog

* Bugfix: The code was overriding the depth texture for overlay cameras, which is incorrect.
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md

* [11.x.x] MSAA fix for Metal MacOS/Editor (#2391)

* Metal MacOS/Editor MSAA fix. Require an explicit MSAA resolve pass unless the platform is mobile

* Added changelog entry
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md

* [11.x.x] Fixed per camera MSAA regression  (#2293)

* Default to XR off when creating the camera and no XR provider is installed.
Added runtime XR on/off check for XR camera.

* Updated CHANGELOG.md and small format fixes.

* Reverted changes in URPCameraEditor and URPCameraData.
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md

* [11.x.x] Early camera color target assignment (#2377)

* Assign the camera color target early in case it is needed during AddRenderPasses

* Update to reflect PR feedback

* Get rid of warnings from accessing cameraColorTarget during AddRenderPasses

* Update CHANGELOG.md
# Conflicts:
#	com.unity.render-pipelines.universal/CHANGELOG.md

Co-authored-by: Elvar Örn Unnþórsson <[email protected]>
Co-authored-by: manuele-bonanno <[email protected]>
Co-authored-by: thomas-zeng <[email protected]>
Co-authored-by: Peter Bay Bastian <[email protected]>
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.

7 participants