Skip to content
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

Add XRCompositor language to accomodate monoscopic displays #140

Merged
merged 7 commits into from
May 29, 2020

Conversation

cabanier
Copy link
Member

  • clarified stereo
  • fixed some typos

Closes #68

@cabanier cabanier requested review from toji and Manishearth May 16, 2020 00:05
@cabanier cabanier marked this pull request as ready for review May 16, 2020 00:07
@Manishearth
Copy link
Contributor

Hmm. This might be confusing, it might be worth erroring if these are used instead of mono?

Alternatively, we can perhaps rename these to not be "stereo" but instead be "views" where they each have the number of subrects equal to the number of views, and we can have getViewport()-esque stuff to get viewports instead of requiring right-left and top-bottom. It seems very weird to me that the core spec is super flexible on this but layers hardcodes the choices.

This would also properly solve #114

@cabanier
Copy link
Member Author

Hmm. This might be confusing, it might be worth erroring if these are used instead of mono?

No, we don't want to error out because then mono experiences will crash unless the author specifically accommodates them.

Alternatively, we can perhaps rename these to not be "stereo" but instead be "views" where they each have the number of subrects equal to the number of views, and we can have getViewport()-esque stuff to get viewports instead of requiring right-left and top-bottom. It seems very weird to me that the core spec is super flexible on this but layers hardcodes the choices.

I agree that stereo doesn't feel right but views is also confusing.
Maybe Natural, all or default?

@Manishearth
Copy link
Contributor

yeah I don't quite care about the naming :)

It seems like we should just have "mono" vs "natural" and natural is laid out however the UA wants, using something like getViewport(). If we do this are the horizontal and vertical variants necessary? I.e. does the user still need the ability to request a particular arrangement?

@cabanier
Copy link
Member Author

yeah I don't quite care about the naming :)

It seems like we should just have "mono" vs "natural" and natural is laid out however the UA wants, using something like getViewport().

Should I change it to natural or should we bring it to the group?

If we do this are the horizontal and vertical variants necessary? I.e. does the user still need the ability to request a particular arrangement?

Yes, those are still needed to accommodate pre-canned video, bitmaps, etc

@cabanier
Copy link
Member Author

/agenda What should we replace the stereo layout with?

@probot-label probot-label bot added the agenda label May 17, 2020
@Manishearth
Copy link
Contributor

Yes, those are still needed to accommodate pre-canned video, bitmaps, etc

Should this not just use the XRMediaBinding stuff?

Even if we decide to have such layers, it still feels weird to then patch mono support in for these layers.

Overall these two seem to b pretty specal-case-y and I'm wary of adding something like that.

@cabanier
Copy link
Member Author

Yes, those are still needed to accommodate pre-canned video, bitmaps, etc

Should this not just use the XRMediaBinding stuff?

No because authors will want to add other effects on top of video or use the stereo bitmaps.

Even if we decide to have such layers, it still feels weird to then patch mono support in for these layers.

Overall these two seem to b pretty specal-case-y and I'm wary of adding something like that.

I'm not adding side-by-side layout in the PR. That was already discussed in previous issues.

@Manishearth
Copy link
Contributor

I'm not adding side-by-side layout in the PR. That was already discussed in previous issues.

Yes, but you are deciding a behavior for side-by-side layout textures in mono sessions, which is what's weird here.

And yeah I think this is a wider discussion that needs to be had: it was kinda ongoing in #114 but that was closed with a specific fix. I'll open something new.

@cabanier
Copy link
Member Author

I'm not adding side-by-side layout in the PR. That was already discussed in previous issues.

Yes, but you are deciding a behavior for side-by-side layout textures in mono sessions, which is what's weird here.

If you have other behavior in mind, we can discuss that here.
Side-by-side layout should not break workflows that happen to be monoscopic.

@Manishearth
Copy link
Contributor

Side-by-side layout should not break workflows that happen to be monoscopic.

Hmm. It still feels weird, but I won't block this. I'll file a separate issue for the existence of left-right and top-bottom.

@cabanier
Copy link
Member Author

Side-by-side layout should not break workflows that happen to be monoscopic.

Hmm. It still feels weird, but I won't block this. I'll file a separate issue for the existence of left-right and top-bottom.

It was introduced by #46. We also discussed it in a weekly call but I don't know how to link that.

@Manishearth
Copy link
Contributor

Yeah, I think there's a bit more to it than that. For example, even if we rename "stereo" there still needs to be a way to get viewports.

@Manishearth
Copy link
Contributor

Hmm, looking at it it seems fine now, but we should probably add non-normative notes explaining these modes better.

@cabanier
Copy link
Member Author

Hmm, looking at it it seems fine now, but we should probably add non-normative notes explaining these modes better.

@Manishearth I added a note. Do you think more information is needed?

@Manishearth
Copy link
Contributor

Looks great, thanks!

@cabanier
Copy link
Member Author

We discussed this change a bit in this week's call but we didn't resolve what name we should use.
I'm leaning towards default

Also, @toji I was wrong when I said a projection layers can be mono on a stereo device. It's defined to be always stereo (aka default) so it has the same number of subimages as the session.

@cabanier
Copy link
Member Author

@toji any comments on my latest changes?

Copy link
Member

@toji toji left a comment

Choose a reason for hiding this comment

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

LGTM!

webxrlayers-1.bs Show resolved Hide resolved
@cabanier cabanier merged commit a5d9c87 into immersive-web:master May 29, 2020
@cabanier cabanier deleted the monovsstereo branch May 29, 2020 22:08
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.

How should layers behave on devices that have only 1 screen
4 participants