Skip to content
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

Uploading textures via ImageData is slower than doing it via regular images #731

Open
adroitwhiz opened this issue Nov 11, 2020 · 2 comments · May be fixed by #732
Open

Uploading textures via ImageData is slower than doing it via regular images #731

adroitwhiz opened this issue Nov 11, 2020 · 2 comments · May be fixed by #732

Comments

@adroitwhiz
Copy link
Contributor

adroitwhiz commented Nov 11, 2020

Expected Behavior

The renderer should do things the fast way

Actual Behavior

In order to upload images' texture data to the GPU (from main memory), we first extract the image data from the image using getImageData, then pass the image data to texImage2D, the function which uploads the data to the GPU.

This change was introduced in #414, because it was supposedly faster than passing the image itself to texImage2D. However, that's only true if the texture is not premultiplied.

Uploading textures to the GPU in WebGL is done with texImage2D, which can be passed (among other things) an <img> element, a <canvas> element, and an ImageData object.

There's a flag, WEBGL_UNPACK_PREMULTIPLY_ALPHA, that controls whether the destination texture data (that ends up on the GPU) is premultiplied or not. Because most of the ways images are stored in the DOM API are relatively "opaque" (getImageData is the only way to get at anything resembling the raw pixel data), the browser will ensure that the destination texture data is premultiplied/not premultiplied (according to your preference) by automatically converting it.

In all browsers I'm aware of, the contents of <img> and <canvas> elements are stored in memory as premultiplied, so if you set WEBGL_UNPACK_PREMULTIPLY_ALPHA to true to indicate that you want premultiplied texture data, no conversion is necessary. However, ImageData objects store un-premultiplied image data for some reason. This means that if you pass an ImageData object to texImage2D, and WEBGL_UNPACK_PREMULTIPLY_ALPHA is true, it will have to premultiply the image data before uploading it, which is slow.

#414 changed things so that instead of passing <canvas> elements to texImage2D, ImageData objects were passed instead. At the time, WEBGL_UNPACK_PREMULTIPLY_ALPHA was false, so the GPU expected non-premultiplied texture data and no conversion was necessary. This was a speed improvement over passing <canvas> elements because the browser would have to *un-*premultiply the texture data there.

However, #515 changed WEBGL_UNPACK_PREMULTIPLY_ALPHA to true, meaning that once again, uploading ImageData to the GPU requires re-premultiplying the data. At the bottom of #515, I actually mention this performance issue as an "unanswered question".

It would be faster to pass the <canvas> or <img> elements directly to texImage2D, obviating the need for any conversion.

Steps to Reproduce

  1. In Firefox, set the webgl.perf.max-warnings preference to -1 (in about:config)
  2. Observe this performance warning:
    image

Operating System and Browser

All (I believe), but easiest to demonstrate on Firefox because of the performance warnings

@adroitwhiz adroitwhiz linked a pull request Nov 11, 2020 that will close this issue
@BryceLTaylor
Copy link

BryceLTaylor commented Nov 12, 2020

@adroitwhiz can a user see or experience this as a problem? What do they see if they do?

@adroitwhiz
Copy link
Contributor Author

This may impact loading performance/RAM usage. #414 was never benchmarked, so I'm not sure how significant it is.

Generation of "mipmaps" for SVG costumes (re-rendering them at higher resolutions) may also become faster if this is fixed.

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

Successfully merging a pull request may close this issue.

2 participants