search

benvanik / WebGL-Inspector

An advanced WebGL debugging toolkit
http://benvanik.github.com/WebGL-Inspector/
BSD 3-Clause "New" or "Revised" License
1.08k stars 160 forks source link

Switch Loader to AMD or import #143

Open greggman opened 7 years ago

greggman commented 7 years ago

Right now there's custom loaders in core/embed.js and core/loader.js. If I switch to AMD style then I think I can get webpack to do the build automatically. Webpack follows the require calls to find all the included modules which means the build script doesn't have to be updated when new things are added. AMD style can also still be used without compiling as well so that seems like a win

I can also switch to use ES style import but that can't be used without a compile step.

Thoughts? Go AMD? Go Import? Leave it as is?

benvanik commented 7 years ago

If Chrome supported import that'd be best (as woo future!), but being able to run uncompiled is nice so AMD is probably best for now. It should be easy enough to switch in the future when browsers start supporting import (I bet someone will have a tool for it :)

greggman commented 7 years ago

Ok, I have should have a PR in a day or 3 that's AMDed. It touches every file and sadly it's after my WebGL2 work so far. Sorry it's not all tiny PRs

One issue I'm having though is AMD at least using require.js loads more async. I might be able to hack it. but ..

  1. Because require.js loads async window.onload gets called before embed.js has had a chance to wrap getContext.

    Note: This should only be an issue for the gliEmbedDebug version.

    I might be able to solve that by hacking require.js or writing my own. Otherwise anytime you use the embedded debug version in an app you'd need to do something to get it to run after embed.js has wrapped getContext. I'm not sure how big a deal that is. Assuming embed.js debug's only real point is debugging maybe that's not an issue? Like I could make embed.js emit an event on the window maybe as one solution if I can't get it to work the same as the current embed

  2. Because it's using require.js I have no idea what issues will come up if the app you're trying to use embed.js in was already using require.js. Hopefully none but if I have to hack require.js to fix #1 above that might be more of an issue. Again I think this only effects when in debug mode (the mode that loads every file separate)

benvanik commented 7 years ago

Restrictions like that in debug mode are fine I think. When packaged it should still work, and that's how most people use it (I'm hoping! debug is slow!)

benvanik commented 7 years ago

Do you think pull #137 will be solved by this AMD change? Or will what it does be even more required?

greggman commented 7 years ago

So just to double check I tried an AMD program

As I suspected you can't use the debug version of the WebGL-Inspector with AMD programs easily. The non-debug versions work fine so I don't think this is an issue. I posted an example here

twgl-amd.html                 // a normal AMD app
twgl-amd-embedded.html        // a version that uses embed.js
twgl-amd-embedded-debug.html  // a version that uses embed.js in debug mode

All of them work in non-debug mode. The last one shows how to get embed.js to work in debug mode with AMD. Unfortunately the debug version of the extension doens't work period and I can't think of any easy work around.

Like I said I don't think that's an issue but I thought it was best to at least check the non-debug version works and document how to get the embed debug version working

greggman commented 7 years ago

Oh yea, and FYI I effectively reverted #116, not all of it, just the parts on HostUI.js, TraceView.js and TraceTab.js.

benvanik commented 7 years ago

Reverting #116 sounds fine to me!