shader-toy
shader-toy copied to clipboard
Enable operating in webworker extension host
Hello from the VS Code team! 👋
I'm a big fan of your work on this extension!
I took a look at what it'd take to get it running in a browser via webworker extension host used by places like github.dev
more info.
In this case, webpack is able to shim some (but not all) node imports to work in a browser, with the biggest exception being fs
. Here I solve that by shimming fs.readFileSync
in the web build with an webpack-inlined import of the webview_base.html
file (all other accesses return the empty string). This worked surprisingly well and makes the extension fully functional for single-file shaders on the desktop webworker extension host, but there are still some issues with loading resources when running in an actual browser.
Imports still don't work, but resolving that is a larger change due to them using the sync variants of the node's fs, which are not implemented on vscode.workspace.fs
.
If this is something you're interested in supporting I'd love to chat more on the finer points of this conversion! Otherwise no worries I can close this out.
Hi Jackson,
This is really cool, appreciate the work! I've been meaning to get back to do some work on this extension, I guess you are giving me the push I needed :D
I'm not familiar with web hosted VSCode, I'll give that a look. It looks like the provided link has extensive information on how to dev and test web extensions, so thanks for that 👍 Perhaps I will understand the issues with imports once I've given it a spin, but maybe you can clarify what imports you mean?
Hey @Malacath-92, thanks for taking a look!
I was able to solve the problem of the the imports, basically the issue was the code was expecting resources needed for the webview (various js libs, images, etc.) to be at a file
-scheme path formed by concatenating the extension's root URI's path with resources
and the name of the item. This works well enough in the desktop case, but in the web the extension's root URI may not be file
scheme itself, so we need to preserve the scheme of the root URI instead of casting it to file
.
This is a lot of talk for a small code change, you can just look here to see the fix :)
Yes, that makes a lot of sense :D I wouldn't be surprised if there are other places in our code that make assumptions about this kinda stuff. We'll see when testing it a bit.
Yes, one bigger sticking point is in loading textures. Running thorough some of the demos, it seems even the textures that simply reference https URI's cause node fs.existsSync and readdirSync calls to be executed, which end up just throwing as I only shimmed readFileSync so far. Reading relative paths also needs some work. But a good number of the demos also work perfectly fine! One import that does work well is self
.
I'm sorry, I thought I would have some time but life decided otherwise. I will pick up the discussion here once I actually get to it.
No worries, I may publish my fork to facilitate selfhosting if that'd be okay by you?
@JacksonKearl I'm sorry for not seeing your message, yes ofc you can do whatever you want. The license is permissive so you do you. I really appreciate your work. And I really hope I'll find the time and energy to take a look at it too.
Posted "Shader Toy (Web)" to the extension marketplace. It's currently designed for web use only and will not activate if it finds the normal shader toy installation active. https://marketplace.visualstudio.com/items?itemName=jakearl.shader-toy-web
Hi @JacksonKearl, I'm hoping to try this a bit over the holidays. Is there anything I should be looking at in particular? I recall you mentioned that the filesystem stuff was not working, but I am not sure I follow whether that makes sense to support on the web-version of VSCode or not.
@Malacath-92 hey sorry I was offline for the holidays. I did go through and categorize the demos that work on web versus those that do not, https://github.com/JacksonKearl/shader-toy/tree/master/demos/web-ready. The central theme with the ones broken tends to be imports of resources, which may be 100% not possible in some cases due to CORS issues, but in other cases (for instance local imports), it should be theoretically possible.
Hi @Malacath-92, I was wondering if you might have any new thoughts on this? We're happy to discuss if you have questions or feedback. Thank you!
Sorry @bamurtaugh I've still been really bad at doing this. I tried once to setup a testing environment for this but failed, I should try again. It always ends up falling lower on my priority list, but at this point I need to stop making excuses. I will give it another try this weekend and if I fail I will look for help.
My main questions at this point:
- how likely will it be that we can do local imports but I will have to guess that I will, Jackson said it would be theoretically possible but I myself am probably not familiar enough with what it would take
- my understand is that the extension could be made to function for both web and desktop in one (when we merge this ofc, I know in the current setup is necessary to have two extensions)
No need to answer these right now, I need to hold up my end of the bargain first 😅
I managed to configure stuff today, my issues were very banal last time. But I am now struggling to debug this extension as the webview hosting VSCode window crashes whenever I hit a breakpoint. I will take a closer look later or tomorrow. But maybe one of you two has an idea what is going?
Took another quick look and from what I can see we will have to wrap all file reads via vscode.workspace.fs
which only supports an async interface. So I guess we'd have to do the long overdue and actually port all file reads to use the async fs interface. Correct me if I am wrong about that, for example if there is some workaround to use the async interface in a sync call.
cc @tanhakabir @aeschli if you have any thoughts on the above comment.
I worked on two commits on-top of this PR, it should be possible to just cherry-pick those. I can later do cleanup when we merge this, we don't have to drag this PR out more than I have already done. The commits in question are
- https://github.com/Malacath-92/shader-toy/commit/d63678cc4ad35913d2ee37edf31c8f4c8ee85f67
- https://github.com/Malacath-92/shader-toy/commit/d4f2ef27bf8a26c6af805a68ddf5c11e707fd302
From my testing everything works other than remote images via URLs. It's also technically unnecessary at this point to shim our own fs in, but again this is cleanup that I could do later.
The last question remaining for me is whether at the state this is at right now, if we deploy the extension, will it work both in desktop and web or is there some additional work we need to do?
Thanks for the great updates, @Malacath-92!
will it work both in desktop and web or is there some additional work we need to do?
There shouldn't be additional work to re-enable an extension for desktop. From the web extension authoring guide:
The web extension runtime is supported on VS Code desktop too. If you decide to create your extension as a web extension, it will be supported on VS Code for the Web (including vscode.dev and github.dev) as well as on the desktop and in services like GitHub Codespaces.
I believe when @JacksonKearl only allowed for web use in his fork above, that was to avoid conflicts between users having both versions of the extension installed, but Jackson can correct me if there is another reason.
To me this is still not clear, I waited for input from Jackson so far, but not sure if it will come (no judgement) Right now as this stands the extension works in web and desktop but there are still features that are broken on both. So some mechanism is still needed to distinguish web or desktop in order to make said features work on desktop while they don't work on web. Plus the work I did in those two commits I linked needs to be integrated into this PR. Afaik I do not have the required permissions to do that myself.
@JacksonKearl it's been a while. Are you eventually coming back to this or should I just close the PR and we keep two separate extensions?
Closing due to inactivity