-
Notifications
You must be signed in to change notification settings - Fork 342
Add a Panorama Video Example #525
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The videoUploading
sample is already using a 360 video. Since the purpose of that sample is just to demonstrate importExternalTexture
, I don't think it would devalue it if we just merged these two samples. Any particular reason to have two?
sample/panorama/meta.ts
Outdated
name: 'Panorama Video', | ||
description: `\ | ||
This example draws a panorama video as a skymap using an external texture. | ||
Video by [Fabio Casati](https://commons.wikimedia.org/wiki/File:Video_360%C2%B0._Timelapse._Bled_Lake_in_Slovenia..webm)`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be sure to attribute correctly. You can get copy-pasteable attribution from Wikimedia if you go to that url and click "Use this file on the web"
<a href="https://commons.wikimedia.org/wiki/File:Video_360%C2%B0._Timelapse._Bled_Lake_in_Slovenia..webm">Fabio Casati</a>, <a href="https://creativecommons.org/licenses/by/3.0">CC BY 3.0</a>, via Wikimedia Commons
... speaking of which, I have no idea where our current 360 video comes from, it has no attribution and might need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure we can combine them.
I'll see if I can find the other video's license. Also, it has a different projectiion than this video but we can have more than 1 video.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Google Lens Gemini identifies it as being at CERN which seems right but I can't find the exact video.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be fine if we just replace that one if we can't find its license.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit. Thank you @greggman!
FYI @mwyrzykowski, I get "Auto-generated layouts mismatch" uncaptured error in Safari Tech Preview with the 360 render pipeline: |
Co-authored-by: François Beaufort <[email protected]>
I noticed the Safari issue too. I tried to repo it in a smaller test but didn't have any luck. I first tried making 2 pipelines, each with a sampler, texture, uniform buffer. # That didn't repo. I then tried the same except with an external texture instead regular texture. # No luck there either. I guess it has something to do with changing videos on the video tag? Maybe between the time it submits the command buffer that uses the external texture, and the time it sets |
@beaufortfrancois @greggman it works if I change line 8620:
which implies WebKit has some incorrect caching. I filed https://bugs.webkit.org/show_bug.cgi?id=296062 Working around this would involve ensuring the entries are not identical between the 360 pipeline and the regular pipeline, but I will fix it next week |
Thanks for looking @mwyrzykowski If you can think of a regression test we can add to the CTS that'd be great! |
No description provided.