Skip to content

Conversation

fcoulombe
Copy link
Contributor

@fcoulombe fcoulombe commented Oct 5, 2021

Please read the Contributing guide before making a PR.

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

Fix auto exposure flickering on the right eye https://fogbugz.unity3d.com/f/cases/1336238/
multipass mode renders with a single eye since it does it in two passes.
using xrActiveEye will ping pong between 0 and 1 but here we always want to pass 0 since we have a single eye.


Testing status

trunk, 2020.3.11f1, 2019.4.30f1
https://unity-ci.cds.internal.unity3d.com/job/9175200
https://unity-ci.cds.internal.unity3d.com/job/9175186
https://unity-ci.cds.internal.unity3d.com/job/9175557


Comments to reviewers

Notes for the reviewers you have assigned.

@fcoulombe fcoulombe marked this pull request as ready for review October 5, 2021 19:54
Copy link
Collaborator

@fabien-unity fabien-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

multipass mode renders with a single eye since it does it in two passes.
using xrActiveEye will ping pong between 0 and 1 but here we always want to pass 0 since we have a single eye.
@fcoulombe fcoulombe force-pushed the xr/graphics/fix-autoexposure-flicker-right-eye branch from 764e803 to ff89028 Compare October 13, 2021 16:40
@fcoulombe fcoulombe requested a review from mikesfbay October 13, 2021 21:03
@fcoulombe fcoulombe requested a review from mikesfbay October 15, 2021 14:42
@fcoulombe fcoulombe requested a review from mikesfbay October 18, 2021 14:19
@mikesfbay
Copy link
Contributor

Can you add a QA to verify (Jason or Ryan)? Otherwise, LGTM

@fcoulombe
Copy link
Contributor Author

Hi @JasonCostanza can you help with testing to see if this fogbugz case is fixed, and there's no regressions with other postfx effects?

@JasonCostanza
Copy link

Hi @JasonCostanza can you help with testing to see if this fogbugz case is fixed, and there's no regressions with other postfx effects?

I'll get on this today or tomorrow if that's okay

@fcoulombe
Copy link
Contributor Author

gentle ping on this

@JasonCostanza
Copy link

Ah, apologies fell off my radar. Putting a note on my monitor to not forget this! Flight Control was down for a while so I lost visibility on some PR's I was assigned, it's back up now

Copy link

@JasonCostanza JasonCostanza left a comment

Choose a reason for hiding this comment

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

2022.1.0a12.1604
Revision: trunk 816252c3efbb
Built: Wed, 06 Oct 2021 17:58:29 GMT

commit 349d581 (HEAD -> xr/graphics/fix-autoexposure-flicker-right-eye, origin/xr/graph

Both eyes render equally on WMR HMD in play mode as described in the bug. LGTM

@fcoulombe fcoulombe dismissed mikesfbay’s stale review October 25, 2021 19:39

this seems to be blocking merge. dismissing since it's outdated

@sebastienlagarde sebastienlagarde merged commit 23837ac into master Nov 3, 2021
@sebastienlagarde sebastienlagarde deleted the xr/graphics/fix-autoexposure-flicker-right-eye branch November 3, 2021 13:45
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.

5 participants