Need ImageBitmap conformance tests that ensure pixel unpack parameters are ignored

[kenrussell]

In Chromium issue http://crbug.com/794706 it was discovered that the WebGL conformance tests test various ImageBitmap creation arguments, but don't verify that the pixel unpack parameters are ignored when uploading ImageBitmaps to WebGL textures.

This needs to be verified.

[greggman]

Just checking but I assume you mean just UNPACK_ALIGNMENT. At least in WebGL2 you need the other parameters if you want to be able to upload parts of an image or if you want to be able to upload a single image as multiple slices of a 3D texture a 2D_ARRAY texture. Otherwise it seems like all the other parameters should be honored including FLIP_Y, PREMULTPLIED_ALPHA, etc...

[kenrussell]

Not for ImageBitmaps. The intent there was that FLIP_Y and PREMULTIPLY_ALPHA must be specified as arguments to createImageBitmap, so that image decoding can happen optimally for the intended usage in WebGL. See this spec:
https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html

[greggman]

There 2 issues here.

1. using ImageBitmap with UNPACK_ROW_LENGTH, UNPACK_IMAGE_HEIGHT, UNPACK_SKIP_PIXELS, UNPACK_SKIP_ROWS, UNPACK_SKIP_IMAGES

those have to continue to work otherwise ImageBitmap WebGL2 apps won't be able to use ImageBitmap to efficently load a 3D or 2D_ARRAY images. Using those parameters a single call to gl.texImage3D can load all slices. Similarly you can slice images into cubemaps

2. Using FLIP_Y, PREMULTIPLIED_ALPHA,

Given these work on the standard non element, arraybuffer based texImage functions I don't get why they shouldn't work on ImageBitmap. Removing them would be especially troublesome for porting to OffscreenCavnas. With them you can just switch your image loading code from using Images to using ImageBitmaps. Without them you have to change even more things. The fact that you can use ImageBitmap efficently by letting it do the conversions async seems irrelevent to what happens when calling texIImage. If you want the efficiency then use the flags in ImageBitmap. If don't care then continue to use the flags in WebGL

[kenrussell]

The main goal in adding support for ImageBitmap was to avoid things like synchronous image re-decodes, which are needed when the developer requests UNPACK_PREMULTIPLY_ALPHA=false and are providing their image data via HTMLImageElement. We do not want to have an unexpected performance cliff for this upload path. Instead we aim to have all browsers support the createImageBitmap creation parameters, and not support these unpack parameters for that case.

We are trying to move away from supporting the FLIP_Y and PREMULTIPLY_ALPHA pixel unpack parameters for ArrayBuffer based texture data. If the user has computed their texture data then they should easily be able to premultiply alpha or not. We are not supporting these for uploads from ArrayBuffer to 3D texture or 2D texture array in WebGL 2.0. (CC'ing @jdashg for any further comments or motivation)

[greggman]

WebGL2 already shipped. Shipping WebGL2 apps already expect FLIP_Y and PREMULTIPLY_ALPHA to work. Arbitrarily pulling that out would break content. WebGL2 supports flipping with ArrayBuffer and has since it shipped.

https://jsfiddle.net/greggman/ucbu2g3t/

I think there are two ways to look at ImageBitmap

1. The main goal is to allow the developer to avoid synchronous image re-decodes

2. The main goal is to force the developer to avoid synchronous image re-decodes

I'd argue the goal should be "allow" not "force"

As an example I just updated twgl to work in workers. As such I had to make the code work with ImageBitmap instead of HTMLImageElement. Assuming all the flags worked made it easy. If the flags don't work not only do I have to add a bunch more code but I'd have to change the twgl API. It becomes extremely non-trival to explain when things get flipped and when they don't

The same will be true of upgrading three.js to run in workers. It's not as simple as not supporting flip_y on ImageBitmap. Most WebGL libraries that load images into textures let you get a handle on the image itself. Many programs use those images to draw into canvas 2D for UI things. With ImageBitmaps supporting WebGL's flip_Y this becomes easy. Without it it becomes hard

 // with flip_Y supported in WebGL
 downloadImageAndUploadToTexture('https://example.com/someimage.png', {flipY: true}, callback);
 
 function callback(err, texture, image) {
    // at this point texture has flipped image
    // but image has non-flipped image which I can use in canvas 2d
 }

Now I try to support running in an worker where I have to use ImageBitmap that code no longer works

 // with flip_Y NOT supported in WebGL (so loading ImageBitmap with flip flag)
 downloadImageAndUploadToTexture('https://example.com/someimage.png', {flipY: true}, callback);
 
 function callback(err, texture, image) {
    // at this point texture has flipped image
    // but image ALSO has flipped image which I can not use in canvas 2d  !!!!!!!!!!!!!!!!!!!!!!!!
    // because it has the wrong orientation  !!!!!
 }

In other words, not supporting these flags on ImageBitmap makes making your code portable between workers and non-workers extremely non-trivial not to mention handling browsers that don't support ImageBitmap at the same time.

I would also argue the bigger goal of ImageBitmap was to provide a way of loading images in workers. a secondary goal would be to allow async image manipulation but not to force it.

[kenrussell]

The synchronous re-decodes of HTMLImageElements when uploading to WebGL textures has been a longstanding and major bottleneck for web applications. ImageBitmap solves these problems by ensuring that the data is ready to be consumed before giving it to the application. The WebGL working group does not want to reintroduce the same performance problems with the ImageBitmap upload path; instead we want to ensure that ImageBitmap works the same in every browser, and both on the main thread and in workers, so that developers can have a high-performance image-to-texture upload path everywhere. This is the path we are pursuing.

[greggman]

Nothing you said in your last comment counters anything I wrote. Being able to avoid those bottlenecks is a fine goal and even with support for the WebGL unpack flags nothing changes. If the developer wants to avoid the bottlenecks they use ImageBitmap and don't use the flags. If the developer just wants to quickly ship their existing app with support for OffscreenCanvas in a workr they can use ImageBitmap and continue to use the flags. Which by the way, based on the fact that they'd be running in a worker anyway means they aren't blocking the main thread. that seems like a win/win.

Because of ease of porting you'll get more apps running their WebGL code in a worker. More apps in a worker means less blocking the main thread. If you make it harder to port and and especially if you make convoluted to support older browsers that still have to use Image then you'll get less adoption, less code running in workers, more main thread blocking.

[greggman]

I'm curious what happened here. I'm guessing you guys went ahead and removed FLIP_Y and PREMULTIPLY_ALPHA support for ImageBitmap under the pretence that you'd be blocking the main thread. Did you also remove conversions from RGB8 / RGBA8 to all the other formats since those would also block the main thread?

As it is the WebGL spec requires a heavy conversion from the image's format to the format and type passed to gl.texImageXX. I argued that blocking usage should be an option or rather that using FLIP_Y and PREMULTIPLY_ALPHA should remain and that if devs want to hit the fast path they should not use them but of they are don't care, or if they are in a worker and so not blocking the main thread, or if they are porting from Image, they should still be able to use those flags. Above the argument was made that FLIP_Y and PREMULTIPLY_ALPHA support was being removed because it blocked the main thread. Well, those format/type conversions also block the main thread so following the same logic should they also be disallowed if "non-blocking" is truly the goal of this API usage?

As it is they seem work (and therefore block for conversion)

Load an JPG via ImageBitmap, upload as RGBA/FLOAT.
https://jsfiddle.net/greggman/bk3taaL9/

The OpenGL ES 3.0 spec makes it clear this conversion does not happen in the driver. It's solely a WebGL spec issue.

The source image data is conceptually first converted to the data type and format specified by the format and type arguments, and then transferred to the WebGL implementation. If a packed pixel format is specified which would imply loss of bits of precision from the image data, this loss of precision must occur.

FLIP_Y is hardly a slower conversion. It's just the difference of an iterator. The premultiply/unpremultiply is slightly heavier but in any case, because of the required format conversion support for the slow path has not been blocked and it seems unjustified to remove support for these flags.

Looking at the ImageBitmap spec it doesn't seem to be clear what format ImageBitmap gets decoded into. So whether or not uploading an image into WebGL is fast (just call gl.texImageXX) or slow (first convert to temporary format, then call gl.texImageXX) seems undefined. For example loading a JPG might be RGB8 but need to be converted to RGBA8 for even the simple case.

[kdashg]

I believed we dropped them to enforce that they should be handled outside the WebGL API, enforcing best-practice.

TBH I'm really tired of playing with these APIs, and would prefer to expose texImage2D(target, level, ImageBitmap/TexImageSource) that just uploads directly ASAP.

Did you also remove conversions from RGB8 / RGBA8 to all the other formats since those would also block the main thread?

I would love to, but the WG has been resistant to removing these from WebGL2, and we can't reasonably remove them from WebGL1 anymore.