Skip to content

Consistently use premultiplied alpha throughout the renderer #460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from

Conversation

adroitwhiz
Copy link
Contributor

@adroitwhiz adroitwhiz commented May 22, 2019

Resolves

Resolves #459

Proposed Changes

This PR changes the codebase to consistently use premultiplied alpha throughout. To be more specific:

  • Skins' textures are now premultiplied when bound. This fixes the alpha blending issue described in more detail in this article.
    • This code added too much duplication to SVGSkin, so I moved its texture-setting code to a new _setTexture method.
  • Skin.hasPremultipliedAlpha has been removed, since all skins now have premultiplied alpha.
  • Because all drawables use premultiplied alpha, gl.blendFunc needn't be set differently for different drawables. It is now called once when the render is initialized, and never again afterwards.
  • The stamp draw mode has been removed. pen transparency fix #418 removed the need for it in the first place.
  • The straightAlpha draw mode has been added. In this mode, the alpha channel is un-premultiplied. This mode is used for extractDrawable, since it is called to extract drawables to 2D canvases whose putImageData function takes un-premultiplied image data. This fixes the dark outline around dragged sprites that currently exists.
  • The line shader now multiplies by alpha.
  • The GPU path for "touching color" now ignores the source sprite's ghost effect. This is necessary because a premultiplied ghost effect will break "touching color" if enabled.
  • Silhouettes still use un-premultiplied alpha. Currently, the PenSkin is the only Skin with a premultiplied silhouette, out of necessity. The other skins set their silhouettes from canvas image data, which is not premultiplied, but PenSkin must set if from framebuffer data, which is. To handle this, the PenSkin is first rendered to a separate "silhouette buffer" using the straightAlpha draw mode, then read into the Silhouette.
    • This fixes a bug introduced in pen transparency fix #418 in which the PenSkin's Silhouette is premultiplied when it shouldn't be, causing e.g. stamped partially-transparent white sprites to trigger "touching color [grey]" blocks. I have added a unit test to check for this bug.
    • Maybe all Silhouettes should use premultiplied alpha. However, this would require premultiplication of every canvas ImageData object passed into bitmap and SVG silhouettes, which I'm not sure how to do quickly and efficiently. Premultiplication of the textures is done via the gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL setting when the texture is bound. We could loop over the pixels in the ImageData buffer or read the data back from the GPU since the conversion is done on the way there, but both of those sound slow.
      • EDIT: After some benchmarking, both CPU and GPU premultiplication paths get ~70-80k pixels/ms on the higher-end machines I've tested, suggesting gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL is just looping over pixels in the CPU as well (it takes up much more time to gl.texImage2D with gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL than it does to read the pixels back). This is still too slow, taking ~50ms at our capped texture size of 2048x2048, but might be worth investigating further. It may be that setting the texture from canvas instead of ImageData is faster (see the Firefox source code) since canvases are (IIRC) premultiplied internally and un-premultiplied when getImageData is called. Reading from the canvas instead of ImageData doubles the GPU path's throughput in both Firefox and Chrome.

Reason for Changes

These changes fix a few different alpha multiplication bugs, as well as somewhat simplifying the codebase-- the GPU always follows the "premultiplied path", Silhouettes always follow the "un-premultiplied path".

@adroitwhiz
Copy link
Contributor Author

adroitwhiz commented May 24, 2019

Alright, so I did some more research.

When copying canvas image data to the GPU, there is a "fast path" and a "slow path".

One of the things that can knock the browser off the "fast path" is if the GPU texture is premultiplied, but the image data is not.

Internally, canvas implementations all use premultiplied alpha. However, getImageData converts to un-premultiplied alpha.

This means that if the GPU texture is supposed to be un-premultiplied, as it was before this PR, the fastest path is through getImageData. However, if we want the texture to be premultiplied, we should pull directly from the canvas.

Should those changes be part of this PR? @cwillisf

@towerofnix
Copy link
Contributor

towerofnix commented Jun 13, 2019

@thisandagain why did you close this PR (and #464)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ugly dark fringe around resampled costume's edges
3 participants