Use premultiplied alpha everywhere, take 2

[adroitwhiz]

Resolves

Resolves #513
Resolves #237

Proposed Changes

• Set gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL before calling texImage2D
  • Move _texture to Skin base class
  • Add _setTexture helper function to Skin base class
  • Disable "automatically attempt mipmaps" in twgl.createTexture
• Use gl.blendFunc(gl.ONE, gl.ONE_MINUS_SRC_ALPHA) for all blending
• Premultiply alpha in ghost effect and pen shader
• Remove stamp draw mode
• Move software-pipeline alpha premultiplication step from RenderWebGL.sampleColor3b to Silhouette.getColor methods
  • Allow Silhouettes to take both non-premultiplied and premultiplied image data by splitting getColor4b into getColor4b and getPremultipliedColor4b

Reason for Changes

• Set gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL before binding textures

This causes textures' alpha channels to be premultiplied before being sent to the GPU. This is necessary for proper blending around edges of sprites; see this article ("GPUs prefer premultiplication") for more info

• Move _texture to Skin base class
• Add _setTexture helper function to Skin base class

These changes reduce code duplication.
_texture was previously initialized by every implementation of Skin, so I moved it to the base class to reduce redundancy.
_setTexture performs the steps of binding the texture, setting and unsetting gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, calling texImage2D, and updating the skin's silhouette. This would otherwise have to be done in every place that sets a Skin implementation's texture to an image.

• Disable "automatically attempt mipmaps" in twgl.createTexture

Because Skin textures are non-power-of-two, no mipmaps can be generated, but for some reason twgl will attempt anyway unless explicitly told not to. This change makes twgl.createTexture throw out tons of glGenerateMipmap warning messages because it is no longer passed a texture (the texture is now set outside the function that creates it).

• Use gl.blendFunc(gl.ONE, gl.ONE_MINUS_SRC_ALPHA) for all blending

This is the proper blend function for compositing premultiplied textures.

• Premultiply alpha in ghost effect and pen shader

Everything should be premultiplied.

• Remove stamp draw mode

The stamp draw mode has not done what it's been said to do ("Draw normally except for pre-multiplied alpha") since #418. It has, in fact, done nothing differently than the default draw mode.

• Move software-pipeline alpha premultiplication step from RenderWebGL.sampleColor3b to Silhouette.getColor methods

As described in the article I linked above, color values must be multiplied by alpha before interpolation to avoid fringing artifacts.

• Allow Silhouettes to take both non-premultiplied and premultiplied image data by splitting getColor4b into getColor4b and getPremultipliedColor4b

A Silhouette can take either data from an image, which is not premultiplied, or data from the GPU, which is premultiplied. If the data does not come premultiplied, we want to premultiply it. If it does come premultiplied, we don't want to multiply it again.

I accomplished this by adding the premultiplied getter/setter to Silhouette. If set to true, it indicates that data passed to the silhouette already comes premultiplied, and if set to false it indicates that it is not premultiplied.

I added a _getColor member to Silhouette which the Silhouette.colorAt functions now call instead of getColor4b. If the silhouette is premultiplied, the member will point towards the getPremultipliedColor4b function. If not, it will point towards the getColor4b function.

This does not appear to regress performance of Silhouette operations.

Unanswered Questions

• It was previously determined that setting textures from ImageData was faster than setting them from HTMLCanvasElements. This may have been because ImageData was not premultiplied, and the textures were not either, allowing the browser to copy the texture data to the GPU directly rather than dividing each individual pixel by its alpha.
  With this PR, textures are now premultiplied, like an HTMLCanvasElement's backing store. So is it faster to set premultiplied textures from a canvas instead of grabbing the ImageData?

• The current shader applies the "ghost" effect (which now multiplies all color values) before the "brightness" and "color" effects (which read color values). This appears to produce correct results, but is probably incorrect. However, I cannot find a way to fix it without spaghettifying the code. How should this be handled? Should the shader divide alpha values if brightness/color are enabled and multiply them again afterwards?

  • Regarding the above, I have decided to divide by alpha values before applying brightness/color effects and multiply afterwards.

• Both in the current code and here, extractDrawable returns a premultiplied image. This produces a dark fringe around dragged sprites. Is this something to fix in the future?

[thisandagain]

/cc @rschamp @mzgoddard

[adroitwhiz]

Pages 20-21 of WebGL Insights go into a bit more detail on the performance impact of UNPACK_PREMULTIPLY_ALPHA_WEBGL.

[cwillisf]

This is awesome - thanks so much -- and thanks for being so thorough! There are a couple things to fix up but we're really excited about this :)

[adroitwhiz]

I hope I've addressed all feedback; thanks for the review!

[adroitwhiz]

P.S. any idea how @mzgoddard benchmarks loading speed consistently? I may try reverting #414 in a follow-up PR per the WebGL Insights analysis I linked earlier.

[cwillisf]

Thanks for the updates!