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

Use iframes #369

Merged
merged 15 commits into from
Mar 7, 2024
Merged

Use iframes #369

merged 15 commits into from
Mar 7, 2024

Conversation

greggman
Copy link
Collaborator

@greggman greggman commented Mar 6, 2024

This PR removes react and next. It allows a sample to be pure JavaScript (though there are none at the moment, all the samples are in typescript). It allows samples to be standalone so that they run separate from the sample selection menu. (which is how they are able to run in iframes).

Note: The last 4 PRs are not merged (they're overwritten). The 3 related to redirects, AFAICT, are irrelevant after this PR. The group descriptions I can add back but this is taking much longer than expect (Have spent ~30hrs) so crossing my fingers this is ok. Open to suggestions.

Note: Originally I had changed all of the xxx.wgsl files to xxx.wgsl.js and surrounded the shaders with

export default `
   ...wgsl...
`;

but I felt there were be resistance so I left it as is. We can revisit that. My issue with it is is it requires a build step and special rules. Of course typescript requires a build step too but if switch the .wgsl to .wgsl.js then all you need is typescript to build and run the examples. You don't need some bundler to hack the .wgsl files into includable files. That seems more useful for people wanting to use the samples elsewhere? OTOH the plugin to make it work is small in most build systems so I'm not 100 sure what I feel about that ATM.

@greggman greggman requested a review from austinEng March 6, 2024 04:07
greggman added 2 commits March 5, 2024 21:25
This PR removes react and next. It allows a sample to be pure JavaScript (though there are none at the moment).
It allows samples to be standalone so that they run separate from the sample selection menu.
greggman added 3 commits March 5, 2024 21:27
personally I'd vote to remove these. They're more clutter than infomative IMO.
@greggman
Copy link
Collaborator Author

greggman commented Mar 6, 2024

there's a live version here: https://greggman.github.io/webgpu-samples-iframes/

public/css/MainLayout.css Outdated Show resolved Hide resolved
sample/animometer/index.html Show resolved Hide resolved
sample/videoUploadingWebCodecs/index.html Show resolved Hide resolved
@austinEng
Copy link
Collaborator

Navigation bar seems cut off on mobile when expanded
greggman github io_webgpu-samples-iframes__sample=helloTriangle(Pixel 7)

On the home page, the background doesn't extend all the way
greggman github io_webgpu-samples-iframes_(Pixel 7)

@greggman
Copy link
Collaborator Author

greggman commented Mar 6, 2024

fixed screenshots

@kainino0x
Copy link
Collaborator

My issue with it is is it requires a build step and special rules.

How about loading the shaders using fetch instead?

@greggman
Copy link
Collaborator Author

greggman commented Mar 6, 2024

How about loading the shaders using fetch instead?

That doesn't seem very idiomatic. I'm only guessing most devs don't want a separate network request for each shader vs just strings in their script. import is a fetch but a bundler fixes that. I guess by that argument bundlers are common and so the small rule to use .wgsl files is not a big deal.

Copy link
Collaborator

@austinEng austinEng left a comment

Choose a reason for hiding this comment

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

this is a huge diff and I tried to look through most of the non-sample parts but may have missed things. I skimmed most of the existing webgpu code.

I think we may as well land it and follow-up on any improvements / changes since the high level structure probably won't change significantly.

index.html Outdated Show resolved Hide resolved
Note: This could be moved to a sample folder now
(could leave a redirect for the previous url)
@greggman greggman merged commit 9a71404 into webgpu:main Mar 7, 2024
1 check passed
@greggman greggman deleted the iframes branch March 7, 2024 03:40
@beaufortfrancois beaufortfrancois mentioned this pull request Mar 7, 2024
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.

4 participants