shader-toy icon indicating copy to clipboard operation
shader-toy copied to clipboard

Enable operating in webworker extension host

Open JacksonKearl opened this issue 2 years ago • 18 comments

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.

JacksonKearl avatar Oct 15 '21 06:10 JacksonKearl

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?

Malacath-92 avatar Oct 18 '21 06:10 Malacath-92

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 :)

JacksonKearl avatar Oct 18 '21 07:10 JacksonKearl

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.

Malacath-92 avatar Oct 18 '21 08:10 Malacath-92

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.

JacksonKearl avatar Oct 18 '21 17:10 JacksonKearl

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.

Malacath-92 avatar Oct 28 '21 09:10 Malacath-92

No worries, I may publish my fork to facilitate selfhosting if that'd be okay by you?

JacksonKearl avatar Oct 28 '21 19:10 JacksonKearl

@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.

Malacath-92 avatar Nov 12 '21 14:11 Malacath-92

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 image

JacksonKearl avatar Nov 19 '21 17:11 JacksonKearl

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 avatar Dec 24 '21 09:12 Malacath-92

@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.

JacksonKearl avatar Jan 04 '22 04:01 JacksonKearl

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!

bamurtaugh avatar Mar 09 '22 00:03 bamurtaugh

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 😅

Malacath-92 avatar Mar 09 '22 08:03 Malacath-92

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?

Malacath-92 avatar Mar 11 '22 08:03 Malacath-92

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.

Malacath-92 avatar Mar 11 '22 11:03 Malacath-92

cc @tanhakabir @aeschli if you have any thoughts on the above comment.

bamurtaugh avatar Mar 11 '22 17:03 bamurtaugh

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?

Malacath-92 avatar Mar 13 '22 10:03 Malacath-92

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.

bamurtaugh avatar Mar 14 '22 16:03 bamurtaugh

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.

Malacath-92 avatar Apr 30 '22 10:04 Malacath-92

@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?

Malacath-92 avatar Oct 20 '22 08:10 Malacath-92

Closing due to inactivity

Malacath-92 avatar Jul 10 '23 07:07 Malacath-92