Bug 203148 – texImage2d fails to set same video frame to multiple textures

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Simon Taylor]

Code such as this in a requestAnimationFrame callback usually results in only the first bound texture being updated:
  
  // video is a HTMLVideoElement
  
  // Update tex1        
  gl.bindTexture(gl.TEXTURE_2D, tex1);
  gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGB, gl.RGB, gl.UNSIGNED_BYTE, video);
  
  // Update tex2 immediately
  gl.bindTexture(gl.TEXTURE_2D, tex2);
  gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGB, gl.RGB, gl.UNSIGNED_BYTE, video);

It looks like WebKit has an optimisation to skip updating a WebGL texture from a video if the frame hasn't changed since the previous texImage2d call.

This is a welcome optimisation but it appears not to verify that the currently bound texture is the same one that was bound for the previous call. There are other cases that should also prevent this optimization (I have not tested these), such as any other glTex[Sub]Image having being called since the last frame upload, the texture having been used as a draw buffer in a Framebuffer object, and perhaps others.

Effectively I suppose the test should be something like:
bool canSkipUpload = (lastWriteActionToBoundTexId == videoUpload && lastVideoUploadFrameToBoundTexId == video.currentFrame);

One use-case for uploading the same video to different textures is to implement pipelined processing of the data in the frame, alternating the texture used for the upload each frame. Something like this:

===
rAF #1:
upload frame 1 to tex 1
start processing data in tex 1

rAF #2:
upload frame 2 to tex 2
start processing data in tex 2
render from tex 1 (frame 1) along with processed result

rAF #3:
upload frame 3 to tex 1
start processing data in tex 1
render from tex 2 (frame 2) along with processed result

etc...

===

Clearly it would also be nice to have an API to query whether the frame has changed before doing the upload, so the whole repeated "upload and process" step can be skipped, but that's a separate issue. I can at least rate-limit the uploads to approximate that, but still need the guarantee that the texture will actually be updated even if the bound texture is different and the frame happens to be the same.

[Simon Taylor]

Created attachment 381291 [details]
Example page

[Simon Taylor]

There is a hosted example here for testing purposes:
https://tango-bravo.net/webkit-bug-203148/index.html

This works fine on Chrome, and shows the broken behaviour in Safari 13 on both MacOS and iOS.

[Radar WebKit Bug Importer]

<rdar://problem/56436745>

[Evan Odabashian]

Just wanted to chime in that we're running into this issue as well.

[Kimmo Kinnunen]

Thanks for the report. I can confirm the issue.

[Kimmo Kinnunen]

*** Bug 234230 has been marked as a duplicate of this bug. ***

[Kimmo Kinnunen]

Created attachment 449655 [details]
Patch

[Kimmo Kinnunen]

Created attachment 449664 [details]
Patch

[Darin Adler]

Comment on attachment 449664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449664&action=review

> Source/WebCore/platform/graphics/cv/GraphicsContextGLCVCocoa.cpp:611
> +    if (it != m_knownContent.end()) {
> +        if (it->value == content) {

Use && instead of nesting?

> Source/WebCore/platform/graphics/cv/GraphicsContextGLCVCocoa.h:71
> +        // TODO: Switch back to UnsafePointer<IOSurfaceRef> once UnsafePointer is safe to compare.

WebKit style uses FIXME for this. Means the same as TODO.

> Source/WebCore/platform/graphics/cv/GraphicsContextGLCVCocoa.h:81
> +        bool operator==(const TextureContent& other) const;

Remove the name "other" please. I also think you might want to add a blank line above this.

[Kimmo Kinnunen]

Created attachment 449786 [details]
Patch for landing

[EWS]

Committed r288434 (246323@main): <https://commits.webkit.org/246323@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449786 [details].

[Simon Taylor]

As a note for people watching this for information on when the fix lands in iOS Safari - this is fixed in iOS 15.4 beta 4.