Enable CopyTextureCHROMIUM in Blink for Tex{Sub}Image2D with more cases

third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 800 matching lines @@
 
   SkSurfaceProps disableLCDProps(0, kUnknown_SkPixelGeometry);
   sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(
       SharedGpuContext::gr(), SkBudgeted::kYes, imageInfo, 0,
       imageInfo.alphaType() == kOpaque_SkAlphaType ? nullptr
                                                    : &disableLCDProps);
   GLuint textureId = skia::GrBackendObjectToGrGLTextureInfo(
                          surface->getTextureHandle(
                              SkSurface::kDiscardWrite_TextureHandleAccess))
                          ->fID;
+  GLenum textureTarget = skia::GrBackendObjectToGrGLTextureInfo(
+                             surface->getTextureHandle(
+                                 SkSurface::kDiscardWrite_TextureHandleAccess))

[Zhenyao Mo, 2017/03/10 00:10:24]

Can we cache this and reuse it for both textureId and textureTarget?

[qiankun, 2017/03/10 02:48:04]

Done.

+                             ->fTarget;
 
   drawingBuffer()->copyToPlatformTexture(
-      gl, textureId, GL_RGBA, GL_UNSIGNED_BYTE, 0, true, false, IntPoint(0, 0),
+      gl, textureTarget, textureId, true, false, IntPoint(0, 0),
       IntRect(IntPoint(0, 0), drawingBuffer()->size()), BackBuffer);
   return surface->makeImageSnapshot();
 }
 
 ImageData* WebGLRenderingContextBase::toImageData(SnapshotReason reason) {
   ImageData* imageData = nullptr;
   // TODO(ccameron): WebGL should produce sRGB images.
   // https://crbug.com/672299
   if (drawingBuffer()) {
     // For un-premultiplied data
@@ skipping 143 matching lines @@
     GL_UNSIGNED_INT_5_9_9_9_REV,
     GL_UNSIGNED_INT_24_8,
     GL_FLOAT_32_UNSIGNED_INT_24_8_REV,
 };
 
 // ES3 enums supported by TexImageSource
 static const GLenum kSupportedTypesTexImageSourceES3[] = {
     GL_HALF_FLOAT, GL_FLOAT, GL_UNSIGNED_INT_10F_11F_11F_REV,
 };
 
-bool isUnsignedIntegerFormat(GLenum internalformat) {
-  switch (internalformat) {
-    case GL_R8UI:
-    case GL_R16UI:
-    case GL_R32UI:
-    case GL_RG8UI:
-    case GL_RG16UI:
-    case GL_RG32UI:
-    case GL_RGB8UI:
-    case GL_RGB16UI:
-    case GL_RGB32UI:
-    case GL_RGBA8UI:
-    case GL_RGB10_A2UI:
-    case GL_RGBA16UI:
-    case GL_RGBA32UI:
-      return true;
-    default:
-      return false;
-  }
-}
-
-bool isSignedIntegerFormat(GLenum internalformat) {
-  switch (internalformat) {
-    case GL_R8I:
-    case GL_R16I:
-    case GL_R32I:
-    case GL_RG8I:
-    case GL_RG16I:
-    case GL_RG32I:
-    case GL_RGB8I:
-    case GL_RGB16I:
-    case GL_RGB32I:
-    case GL_RGBA8I:
-    case GL_RGBA16I:
-    case GL_RGBA32I:
-      return true;
-    default:
-      return false;
-  }
-}
-
-bool isIntegerFormat(GLenum internalformat) {
-  return (isUnsignedIntegerFormat(internalformat) ||
-          isSignedIntegerFormat(internalformat));
-}
-
-bool isFloatType(GLenum type) {
-  switch (type) {
-    case GL_FLOAT:
-    case GL_HALF_FLOAT:
-    case GL_HALF_FLOAT_OES:
-    case GL_UNSIGNED_INT_10F_11F_11F_REV:
-      return true;
-    default:
-      return false;
-  }
-}
-
 bool isSRGBFormat(GLenum internalformat) {
   switch (internalformat) {
     case GL_SRGB_EXT:
     case GL_SRGB_ALPHA_EXT:
     case GL_SRGB8:
     case GL_SRGB8_ALPHA8:
       return true;
     default:
       return false;
   }
@@ skipping 3891 matching lines @@
                                            ExceptionState& exceptionState) {
   texImageHelperHTMLImageElement(TexImage2D, target, level, internalformat,
                                  format, type, 0, 0, 0, image,
                                  sentinelEmptyRect(), 1, 0, exceptionState);
 }
 
 bool WebGLRenderingContextBase::canUseTexImageByGPU(
     TexImageFunctionID functionID,
     GLint internalformat,
     GLenum type) {
-  if (functionID == TexImage2D &&
-      (isFloatType(type) || isIntegerFormat(internalformat) ||
-       isSRGBFormat(internalformat)))
+  // SRGB format has color-space conversion issue, see
+  // https://github.com/KhronosGroup/WebGL/issues/2165.
+  if (isSRGBFormat(internalformat))
     return false;
-  // TODO(crbug.com/622958): Implement GPU-to-GPU path for WebGL 2 and more
-  // internal formats.
-  if (functionID == TexSubImage2D &&
-      (isWebGL2OrHigher() || extensionEnabled(OESTextureFloatName) ||
-       extensionEnabled(OESTextureHalfFloatName) ||
-       extensionEnabled(EXTsRGBName)))
+  // Before we can detect internalformat in TexSubImage2D, we disable GPU path
+  // if EXT_sRGB extension is enabled.
+  if (functionID == TexSubImage2D && extensionEnabled(EXTsRGBName))

[Zhenyao Mo, 2017/03/10 00:10:24]

This is a big fallback.  Can we look into lifting this?

[qiankun, 2017/03/10 02:48:04]

When we get clear on how to handle sRGB texture, we can think about lifting
this. Add a TODO here.

     return false;
+#if OS(MACOSX)
+  // RGB5_A1 is not color-renderable on NVIDIA Mac, see crbug.com/676209.
+  if (type == GL_UNSIGNED_SHORT_5_5_5_1)
+    return false;
+#endif
+  // OES_texture_half_float doesn't support HALF_FLOAT_OES type for
+  // CopyTexImage/CopyTexSubImage. And OES_texture_half_float doesn't require
+  // HALF_FLOAT_OES type texture to be renderable. So, HALF_FLOAT_OES type
+  // texture cannot be copied to or drawn to by glCopyTextureCHROMIUM.
+  if (type == GL_HALF_FLOAT_OES)
+    return false;
+
   return true;
 }
 
 SnapshotReason WebGLRenderingContextBase::functionIDToSnapshotReason(
     TexImageFunctionID id) {
   switch (id) {
     case TexImage2D:
       return SnapshotReasonWebGLTexImage2D;
     case TexSubImage2D:
       return SnapshotReasonWebGLTexSubImage2D;
     case TexImage3D:
       return SnapshotReasonWebGLTexImage3D;
     case TexSubImage3D:
       return SnapshotReasonWebGLTexSubImage3D;
   }
   NOTREACHED();
   return SnapshotReasonUnknown;
 }
 
 void WebGLRenderingContextBase::texImageCanvasByGPU(
     TexImageFunctionID functionID,
     HTMLCanvasElement* canvas,
+    GLenum target,
     GLuint targetTexture,
-    GLenum targetInternalformat,
-    GLenum targetType,
-    GLint targetLevel,
     GLint xoffset,
     GLint yoffset,
     const IntRect& sourceSubRectangle) {
   if (!canvas->is3D()) {
     ImageBuffer* buffer = canvas->buffer();
     if (buffer &&
         !buffer->copyToPlatformTexture(
-            functionIDToSnapshotReason(functionID), contextGL(), targetTexture,
-            targetInternalformat, targetType, targetLevel,
-            m_unpackPremultiplyAlpha, m_unpackFlipY, IntPoint(xoffset, yoffset),
-            sourceSubRectangle)) {
+            functionIDToSnapshotReason(functionID), contextGL(), target,
+            targetTexture, m_unpackPremultiplyAlpha, m_unpackFlipY,
+            IntPoint(xoffset, yoffset), sourceSubRectangle)) {
       NOTREACHED();
     }
   } else {
     WebGLRenderingContextBase* gl =
         toWebGLRenderingContextBase(canvas->renderingContext());
     ScopedTexture2DRestorer restorer(gl);
     if (!gl->drawingBuffer()->copyToPlatformTexture(
-            contextGL(), targetTexture, targetInternalformat, targetType,
-            targetLevel, m_unpackPremultiplyAlpha, !m_unpackFlipY,
-            IntPoint(xoffset, yoffset), sourceSubRectangle, BackBuffer)) {
+            contextGL(), target, targetTexture, m_unpackPremultiplyAlpha,
+            !m_unpackFlipY, IntPoint(xoffset, yoffset), sourceSubRectangle,
+            BackBuffer)) {
       NOTREACHED();
     }
   }
 }
 
 void WebGLRenderingContextBase::texImageByGPU(
     TexImageFunctionID functionID,
     WebGLTexture* texture,
     GLenum target,
     GLint level,
-    GLint internalformat,
-    GLenum type,
     GLint xoffset,
     GLint yoffset,
     GLint zoffset,
     CanvasImageSource* image,
     const IntRect& sourceSubRectangle) {
   DCHECK(image->isCanvasElement() || image->isImageBitmap());
   int width = sourceSubRectangle.width();
   int height = sourceSubRectangle.height();
 
   ScopedTexture2DRestorer restorer(this);
 
   GLuint targetTexture = texture->object();
-  GLenum targetType = type;
-  GLenum targetInternalformat = internalformat;
-  GLint targetLevel = level;
   bool possibleDirectCopy = false;
-  if (functionID == TexImage2D) {
-    possibleDirectCopy = Extensions3DUtil::canUseCopyTextureCHROMIUM(
-        target, internalformat, type, level);
+  if (functionID == TexImage2D || functionID == TexSubImage2D) {
+    possibleDirectCopy = Extensions3DUtil::canUseCopyTextureCHROMIUM(target);
   }
 
   GLint copyXOffset = xoffset;
   GLint copyYOffset = yoffset;
+  GLenum copyTarget = target;
 
   // if direct copy is not possible, create a temporary texture and then copy
   // from canvas to temporary texture to target texture.
   if (!possibleDirectCopy) {
-    targetLevel = 0;
-    targetInternalformat = GL_RGBA;
-    targetType = GL_UNSIGNED_BYTE;
     contextGL()->GenTextures(1, &targetTexture);
     contextGL()->BindTexture(GL_TEXTURE_2D, targetTexture);
     contextGL()->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,
                                GL_NEAREST);
     contextGL()->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
                                GL_NEAREST);
     contextGL()->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S,
                                GL_CLAMP_TO_EDGE);
     contextGL()->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T,
                                GL_CLAMP_TO_EDGE);
-    contextGL()->TexImage2D(GL_TEXTURE_2D, 0, targetInternalformat, width,
-                            height, 0, GL_RGBA, targetType, 0);
+    contextGL()->TexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0,
+                            GL_RGBA, GL_UNSIGNED_BYTE, 0);
     copyXOffset = 0;
     copyYOffset = 0;
+    copyTarget = GL_TEXTURE_2D;
   }
 
   if (image->isCanvasElement()) {
     texImageCanvasByGPU(functionID, static_cast<HTMLCanvasElement*>(image),
-                        targetTexture, targetInternalformat, targetType,
-                        targetLevel, copyXOffset, copyYOffset,
+                        copyTarget, targetTexture, copyXOffset, copyYOffset,
                         sourceSubRectangle);
   } else {
-    texImageBitmapByGPU(static_cast<ImageBitmap*>(image), targetTexture,
-                        targetInternalformat, targetType, targetLevel,
-                        !m_unpackFlipY);
+    texImageBitmapByGPU(static_cast<ImageBitmap*>(image), copyTarget,
+                        targetTexture, !m_unpackFlipY, copyXOffset, copyYOffset,
+                        sourceSubRectangle);
   }
 
   if (!possibleDirectCopy) {
     GLuint tmpFBO;
     contextGL()->GenFramebuffers(1, &tmpFBO);
     contextGL()->BindFramebuffer(GL_FRAMEBUFFER, tmpFBO);
     contextGL()->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
                                       GL_TEXTURE_2D, targetTexture, 0);
     contextGL()->BindTexture(texture->getTarget(), texture->object());
     if (functionID == TexImage2D) {
@@ skipping 79 matching lines @@
     // The GPU-GPU copy path uses the Y-up coordinate system.
     IntRect adjustedSourceSubRectangle = sourceSubRectangle;
     if (!m_unpackFlipY) {
       adjustedSourceSubRectangle.setY(canvas->height() -
                                       adjustedSourceSubRectangle.maxY());
     }
 
     if (functionID == TexImage2D) {
       texImage2DBase(target, level, internalformat, sourceSubRectangle.width(),
                      sourceSubRectangle.height(), 0, format, type, 0);
-      texImageByGPU(functionID, texture, target, level, internalformat, type, 0,
-                    0, 0, canvas, adjustedSourceSubRectangle);
+      texImageByGPU(functionID, texture, target, level, 0, 0, 0, canvas,
+                    adjustedSourceSubRectangle);
     } else {
-      texImageByGPU(functionID, texture, target, level, GL_RGBA, type, xoffset,
-                    yoffset, 0, canvas, adjustedSourceSubRectangle);
+      texImageByGPU(functionID, texture, target, level, xoffset, yoffset, 0,
+                    canvas, adjustedSourceSubRectangle);
     }
   } else {
     // 3D functions.
 
     // TODO(zmo): Implement GPU-to-GPU copy path (crbug.com/612542).
     // Note that code will also be needed to copy to layers of 3D
     // textures, and elements of 2D texture arrays.
     texImageImpl(
         functionID, target, level, internalformat, xoffset, yoffset, zoffset,
         format, type, canvas
@@ skipping 61 matching lines @@
   if (!validateTexFunc(funcName, functionType, SourceHTMLVideoElement, target,
                        level, internalformat, video->videoWidth(),
                        video->videoHeight(), 1, 0, format, type, xoffset,
                        yoffset, zoffset))
     return;
 
   bool sourceImageRectIsDefault =
       sourceImageRect == sentinelEmptyRect() ||
       sourceImageRect ==
           IntRect(0, 0, video->videoWidth(), video->videoHeight());
+  // Format of source video may be 16-bit format, e.g. Y16 format.
+  // glCopyTextureCHROMIUM will cause precision lost when uploading such video
+  // texture to half float or float texture.

[Zhenyao Mo, 2017/03/10 00:10:24]

Well, the same can be said for RGBA8 formats, so I am not sure if we want to put
in this limit.

[qiankun, 2017/03/10 02:48:04]

Or we can increase the tolerance in the tests? I met this issue for
depth_capture_tests where tolerance == 1.5/65535 :
https://cs.chromium.org/chromium/src/content/test/data/media/getusermedia-dep....

[Zhenyao Mo, 2017/03/10 22:45:43]

What will be the tolerance to make it pass? I think slightly increase the
tolerance is fine if the benefit is a much faster path.

[qiankun, 2017/03/10 23:54:19]

When calling glCopy{Sub}TextureCHROMIUM, source_internal_format of the source
texture in this case is BGRA8 or RGBA8 on different OSes. So, we should set the
tolerance to 1.0 / 255 to make the test pass.

[aleksandar.stojiljkovic, 2017/03/14 13:43:46]

Thanks.
Yes, that would break the 16 bit video. Another work that could be affected is
HDR video - hubbe@ has started work on HDR video decoding
(https://crbug.com/682416) and I'm planning to work on support for GL_RGB10_A2.

Could we try this as a fix (so that we also enable copy texture for
isFloatType(type) case):
[L.5213-L.5217] is guarding two things:
1. copyVideoTextureToPlatformTexture (no precision loss, I think)
2. accelerated image buffer (painting to RGBA is introducing cutting precision
to 8 bits).
So, would splitting 1. and 2. and inserting 16-bit video code on L.5269 between
them fix this?
I remember doing that [1] but changed my mind later to avoid introducing
unnecessary change:
[1]
L.5095
https://codereview.chromium.org/2476693002/diff/100001/third_party/WebKit/Sou...





 
 

[qiankun, 2017/03/31 09:54:05]

Done as aleksandar suggested.

   if (functionID == TexImage2D && sourceImageRectIsDefault && depth == 1 &&
-      GL_TEXTURE_2D == target && Extensions3DUtil::canUseCopyTextureCHROMIUM(
-                                     target, internalformat, type, level)) {
+      GL_TEXTURE_2D == target &&
+      canUseTexImageByGPU(functionID, internalformat, type) &&
+      (type != GL_FLOAT && type != GL_HALF_FLOAT &&
+       type != GL_UNSIGNED_INT_10F_11F_11F_REV)) {
     DCHECK_EQ(xoffset, 0);
     DCHECK_EQ(yoffset, 0);
     DCHECK_EQ(zoffset, 0);
     // Go through the fast path doing a GPU-GPU textures copy without a readback
     // to system memory if possible.  Otherwise, it will fall back to the normal
     // SW path.
 
     // Note that neither
     // HTMLVideoElement::copyVideoTextureToPlatformTexture nor
     // ImageBuffer::copyToPlatformTexture allocate the destination texture
@@ skipping 21 matching lines @@
         // implementation to do any necessary color space conversion on the GPU
         // (though it may still do a CPU conversion and upload the results).
         video->paintCurrentFrame(
             imageBuffer->canvas(),
             IntRect(0, 0, video->videoWidth(), video->videoHeight()), nullptr);
 
         // This is a straight GPU-GPU copy, any necessary color space conversion
         // was handled in the paintCurrentFrameInContext() call.
 
         if (imageBuffer->copyToPlatformTexture(
-                functionIDToSnapshotReason(functionID), contextGL(),
-                texture->object(), internalformat, type, level,
-                m_unpackPremultiplyAlpha, m_unpackFlipY, IntPoint(0, 0),
+                functionIDToSnapshotReason(functionID), contextGL(), target,
+                texture->object(), m_unpackPremultiplyAlpha, m_unpackFlipY,
+                IntPoint(0, 0),
                 IntRect(0, 0, video->videoWidth(), video->videoHeight()))) {
           return;
         }
       }
     }
   }
 
   if (sourceImageRectIsDefault) {
     // Try using optimized CPU-GPU path for some formats: e.g. Y16 and Y8. It
     // leaves early for other formats or if frame is stored on GPU.
@@ skipping 11 matching lines @@
   RefPtr<Image> image = videoFrameToImage(video);
   if (!image)
     return;
   texImageImpl(functionID, target, level, internalformat, xoffset, yoffset,
                zoffset, format, type, image.get(),
                WebGLImageConversion::HtmlDomVideo, m_unpackFlipY,
                m_unpackPremultiplyAlpha, sourceImageRect, depth,
                unpackImageHeight);
 }
 
-void WebGLRenderingContextBase::texImageBitmapByGPU(ImageBitmap* bitmap,
-                                                    GLuint targetTexture,
-                                                    GLenum targetInternalformat,
-                                                    GLenum targetType,
-                                                    GLint targetLevel,
-                                                    bool flipY) {
-  bitmap->bitmapImage()->copyToTexture(drawingBuffer()->contextProvider(),
-                                       targetTexture, targetInternalformat,
-                                       targetType, flipY);
+void WebGLRenderingContextBase::texImageBitmapByGPU(
+    ImageBitmap* bitmap,
+    GLenum target,
+    GLuint targetTexture,
+    bool flipY,
+    GLint xoffset,
+    GLint yoffset,
+    const IntRect& sourceSubRectangle) {
+  bitmap->bitmapImage()->copyToTexture(
+      drawingBuffer()->contextProvider(), target, targetTexture, flipY,
+      IntPoint(xoffset, yoffset), sourceSubRectangle);
 }
 
 void WebGLRenderingContextBase::texImage2D(GLenum target,
                                            GLint level,
                                            GLint internalformat,
                                            GLenum format,
                                            GLenum type,
                                            HTMLVideoElement* video,
                                            ExceptionState& exceptionState) {
   texImageHelperHTMLVideoElement(TexImage2D, target, level, internalformat,
@@ skipping 47 matching lines @@
   ASSERT(bitmap->bitmapImage());
 
   // TODO(kbr): make this work for sub-rectangles of ImageBitmaps.
   if (functionID != TexSubImage3D && functionID != TexImage3D &&
       bitmap->isAccelerated() &&
       canUseTexImageByGPU(functionID, internalformat, type) &&
       !selectingSubRectangle) {
     if (functionID == TexImage2D) {
       texImage2DBase(target, level, internalformat, width, height, 0, format,
                      type, 0);
-      texImageByGPU(functionID, texture, target, level, internalformat, type, 0,
-                    0, 0, bitmap, sourceSubRect);
+      texImageByGPU(functionID, texture, target, level, 0, 0, 0, bitmap,
+                    sourceSubRect);
     } else if (functionID == TexSubImage2D) {
-      texImageByGPU(functionID, texture, target, level, GL_RGBA, type, xoffset,
-                    yoffset, 0, bitmap, sourceSubRect);
+      texImageByGPU(functionID, texture, target, level, xoffset, yoffset, 0,
+                    bitmap, sourceSubRect);
     }
     return;
   }
   sk_sp<SkImage> skImage = bitmap->bitmapImage()->imageForCurrentFrame();
   SkPixmap pixmap;
   uint8_t* pixelDataPtr = nullptr;
   RefPtr<Uint8Array> pixelData;
   // In the case where an ImageBitmap is not texture backed, peekPixels() always
   // succeed.  However, when it is texture backed and !canUseTexImageByGPU, we
   // do a GPU read back.
@@ skipping 2400 matching lines @@
 
 void WebGLRenderingContextBase::getHTMLOrOffscreenCanvas(
     HTMLCanvasElementOrOffscreenCanvas& result) const {
   if (canvas())
     result.setHTMLCanvasElement(canvas());
   else
     result.setOffscreenCanvas(offscreenCanvas());
 }
 
 }  // namespace blink