Cache shaders

[mikolalysenko]

This new version of gl-shader-core caches shaders and attribute locations making it faster to assign values to attributes.

It also fixes a bug which existed previously where the location of some attributes could get screwed up during rebinding.

Compatibility-wise, this would be a major version bump since the reference to the underlying shader and program objects are dropped.

Another major change is that dispose() no longer releases the shader. This is necessary in order for the caching to have some meaningful benefit. I think for shaders this is ultimately a worthwhile tradeoff as:

1. Applications tend to allocate a few shaders which persist through the lifetime of the whole app (so dispose is rarely ever called)
2. Shaders use an insignificant amount of memory on the GPU compared to textures or vertex buffers
3. Shader compilation/linking is expensive

[hughsk]

@mikolalysenko this looks great!

I'm not 100% sure if it should be the "core" though. There's cases to consider when shaders are editable at runtime, e.g. shadertoy and its ilk, but also developer tooling such as glslify-live, where you're potentially getting shaders updated and recompiled every few seconds. I'm not sure how much memory that would leak in practice, but could add a lot of noise when using WebGL Inspector. Maybe could solve this with an LRU cache?

The nice thing about the low-level packages is that they're just simple convenience wrappers around the API and you can layer your own stuff on top as needed. I reckon either this PR should become gl-shader-cached, or we keep the original around as gl-shader-raw and point people from one to the other somewhere up the top of the README. Up to you on which you'd prefer :)

[mikolalysenko]

It's true, though the real problem here is the attribute location stuff. If you are swapping around attribute locations it requires a relink on the shader which is expensive.

I could implement a ref counting scheme and have it dispose the shader if the count goes below some threshold maybe?

[hughsk]

If I understand correctly, dispose() would then still work as it did previously, right? That'd be perfect :)

[mikolalysenko]

Ok, I added a reference counting mechanism to the shaders which should handle resource freeing if you release a shader. Now .dispose() works as expected and releases all resources associated with a shader.

Multiple program objects are still created (which is actually a good thing since it makes it faster to update attribute locations), but they are destroyed as expected when the shader is disposed().

[mattdesl]

Looks aweomse 👍 But I'm also wondering whether this might be better suited as a separate module? So we can still have a "dumb and simple" shader wrapper for situations where it may be warranted.

Main concerns:

• code complexity / maintainability
• goes against unix philosophy of "doing one thing" -- now it's a shader wrapper and shader caching system
• currently most of the modules like gl-fbo, gl-buffer, gl-texture2d are wrappers around a single underlying WebGL object, which may lead to some confusion
• engines might choose to handle attribute locations differently; e.g. using aliases for mesh attributes so multiple meshes with slightly different layouts can still utilize the same underlying shader program. In these cases a caching system is unnecessary and potentially problematic

[mikolalysenko]

Complexity is a concern, but this is a fairly modest increase and all of this behavior is under the hood so to speak.

Regarding caching of shaders/program objects: I can't see the behavior being problematic in any way. The reason for this is that shader and program objects are effectively immutable after creation. That is, they have no properties beyond their source code, attachments and attribute locations and these can't change after they are compiled/linked. Caching them solves a pretty horrible issue with the attribute location behavior (ie it didn't work before) and has the added benefit of giving a performance improvement in some usages.

As to whether this is truly a unixy approach, I won't comment. However, it should be noted that gl-shader-core has never really done the 1 webgl object - 1 wrapper object, since it combines the underlying webgl shader + program objects into a common interface. The reason for this is that it is pretty cumbersome to create shaders and then link them separately. Removing this annoying step still seems like a net win to me.

If an engine also wants to handle attribute aliasing themselves within their own shaders, then this approach should still work and not create any problems.

[hughsk]

I'm with @mattdesl on this one, after giving it some more thought while this looks awesome but might still be a little too magical alongside the other "WebGL core" packages. Though there's workarounds, removing .handle et al does break some of my things and may also break others'.

I'd prefer this as a separate package but if not, we should definitely still have the original functionality available somewhere. Note also that the new glslify will make it easy to switch between adapters, so ultimately this boils down to naming things :)

In that case, maybe we find another name for this package, and repurpose gl-shader to do what gl-shader-core does currently? Happy to use the cached version with glslify for the time being.

[mikolalysenko]

Regarding .handle, this could be reverted. The underlying motivation for the change in naming was that it is a bit more descriptive to use .program.

But the caching is really necessary to get the attribute locations working consistently. The problem is that most WebGL implementations forget the order of vertex attribute locations upon a relink, so you need to save that at minimum. Moreover, if you switch attribute locations on a shader it can cause performance issues which are hard to understand or predict otherwise.

In terms of added complexity, I don't think the changes are particularly complex. All of the additional logic is in this file:

https://github.com/stackgl/gl-shader-core/blob/cache-shaders/lib/shader-cache.js

Taking away the extra stuff required for shader allocations, the difference is maybe 50 lines.

And I should stress that this has no observable effect outside the shader core object. Programs still needed to be rebuilt after changing attribute locations, so the semantics of the module are still intact. Caching shaders doesn't seem to have any real technical downside.

[mikolalysenko]

I would like to see this change set get merged quickly because it solves a bunch of problems relating to attribute locations.

Regarding shader reloading I have a proposal:

What if we replace the currently (awkwardly named) and not publically documented updateExports API with a new method called replaceSource or something which would take as input a new vertex and fragment shader source strings and rebuild the shader in place?

This would be simpler for doing live reloading than the currently grungy system that requires reaching into the guts of the object and doing some hacky live attribute swapping.

What do you guys think about this?

[mattdesl]

Support for reloading would be good. 👍

I'm fine with this being merged. At some point I'll start porting my sprite batching/text rendering back to gl-vao, which I was avoiding because of attribute location woes.

If we later find ourselves wanting a non-caching shader wrapper then it can always be pulled out of this module.