KEMBAR78
Short circuit updateRenderState() if no render state is passed by Manishearth · Pull Request #1031 · immersive-web/webxr · GitHub
Skip to content

Conversation

Manishearth
Copy link
Contributor

@Manishearth Manishearth commented May 11, 2020

Fixes #880


Preview | Diff

@Manishearth Manishearth requested a review from toji May 11, 2020 21:45
1. If |newState|'s {{XRRenderStateInit/baseLayer}}'s was created with an {{XRSession}} other than |session|, throw an {{InvalidStateError}} and abort these steps.
1. If |newState|'s {{XRRenderStateInit/baseLayer}} was created with an {{XRSession}} other than |session|, throw an {{InvalidStateError}} and abort these steps.
1. If |newState|'s {{XRRenderStateInit/inlineVerticalFieldOfView}} is set and |session| is an [=immersive session=], throw an {{InvalidStateError}} and abort these steps.
1. If none of |newState|'s {{XRRenderStateInit/depthNear}}, {{XRRenderStateInit/depthFar}}, {{XRRenderStateInit/inlineVerticalFieldOfView}}, {{XRRenderStateInit/baseLayer}}, {{XRRenderStateInit/layers}} are set, abort these steps.
Copy link
Member

Choose a reason for hiding this comment

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

I'm slightly concerned that this lends itself to accidentally missing future additions to the dictionary, but given that partial dictionaries are apparently verboten I guess that's not as bad as it could be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was my concern as well, unfortunately since dictionaries aren't real you can't just "check if it's empty"

@Manishearth Manishearth force-pushed the updaterenderstate-empty branch from 5825859 to fa56fc2 Compare May 11, 2020 22:03
@Manishearth
Copy link
Contributor Author

Rebased

@Manishearth Manishearth merged commit fb2ff08 into immersive-web:master May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Corrections and clarifications for description/steps for updateRenderState()

2 participants