Bug 1479145 - Handle arbitrary strides for WebGL-to-SharedSurface readback on platforms that support it. r=jgilbert
authorMarkus Stange <mstange@themasta.com>
Fri, 29 Mar 2019 20:18:53 +0000
changeset 528809 d5836acadff957f68bbac51a82d9bcded0af5f56
parent 528808 0ea914bf03b719cd0821564b551dbb384ba95ff5
child 528810 8acf628be0368eebd5b06b1480de6cad8deb0ca4
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgilbert
bugs1479145, 1540209
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1479145 - Handle arbitrary strides for WebGL-to-SharedSurface readback on platforms that support it. r=jgilbert The only platforms that do not support GL_PACK_ROW_LENGTH are platforms with GLES 2. So on those platforms, trying to read back into buffers whose stride is not width * 4 will assert. That's fine because we usually don't encounter buffers with such large strides on GLES 2 platforms. The only platform that really needs to handle the large strides is macOS, and it always supports GL_PACK_ROW_LENGTH. On macOS, we often run into large strides on surfaces that we intend to upload as textures at some point, because large stride alignments are required for efficient upload performance on some drivers. Bug 1540209 tracks fixing the general case. Differential Revision: https://phabricator.services.mozilla.com/D25464
gfx/gl/ScopedGLHelpers.cpp
gfx/gl/ScopedGLHelpers.h
gfx/gl/SharedSurface.cpp
gfx/layers/client/CanvasClient.cpp
--- a/gfx/gl/ScopedGLHelpers.cpp
+++ b/gfx/gl/ScopedGLHelpers.cpp
@@ -382,16 +382,32 @@ ScopedPackState::ScopedPackState(GLConte
   mGL->fGetIntegerv(LOCAL_GL_PACK_SKIP_ROWS, &mSkipRows);
 
   if (mPixelBuffer != 0) mGL->fBindBuffer(LOCAL_GL_PIXEL_PACK_BUFFER, 0);
   if (mRowLength != 0) mGL->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, 0);
   if (mSkipPixels != 0) mGL->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, 0);
   if (mSkipRows != 0) mGL->fPixelStorei(LOCAL_GL_PACK_SKIP_ROWS, 0);
 }
 
+bool ScopedPackState::SetForWidthAndStrideRGBA(GLsizei aWidth,
+                                               GLsizei aStride) {
+  MOZ_ASSERT(aStride % 4 == 0, "RGBA data should always be 4-byte aligned");
+  MOZ_ASSERT(aStride / 4 >= aWidth, "Stride too small");
+  if (aStride / 4 == aWidth) {
+    // No special handling needed.
+    return true;
+  }
+  if (mGL->HasPBOState()) {
+    // HasPBOState implies support for GL_PACK_ROW_LENGTH.
+    mGL->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, aStride / 4);
+    return true;
+  }
+  return false;
+}
+
 void ScopedPackState::UnwrapImpl() {
   mGL->fPixelStorei(LOCAL_GL_PACK_ALIGNMENT, mAlignment);
 
   if (!mGL->HasPBOState()) return;
 
   mGL->fBindBuffer(LOCAL_GL_PIXEL_PACK_BUFFER, mPixelBuffer);
   mGL->fPixelStorei(LOCAL_GL_PACK_ROW_LENGTH, mRowLength);
   mGL->fPixelStorei(LOCAL_GL_PACK_SKIP_PIXELS, mSkipPixels);
--- a/gfx/gl/ScopedGLHelpers.h
+++ b/gfx/gl/ScopedGLHelpers.h
@@ -282,16 +282,19 @@ struct ScopedPackState : public ScopedGL
   GLuint mPixelBuffer;
   GLint mRowLength;
   GLint mSkipPixels;
   GLint mSkipRows;
 
  public:
   explicit ScopedPackState(GLContext* gl);
 
+  // Returns whether the stride was handled successfully.
+  bool SetForWidthAndStrideRGBA(GLsizei aWidth, GLsizei aStride);
+
  protected:
   void UnwrapImpl();
 };
 
 struct ResetUnpackState : public ScopedGLWrapper<ResetUnpackState> {
   friend struct ScopedGLWrapper<ResetUnpackState>;
 
  protected:
--- a/gfx/gl/SharedSurface.cpp
+++ b/gfx/gl/SharedSurface.cpp
@@ -503,24 +503,19 @@ bool ReadbackSharedSurface(SharedSurface
     GLContext* gl = src->mGL;
     GetActualReadFormats(gl, dstGLFormat, dstType, &readGLFormat, &readType);
 
     MOZ_ASSERT(readGLFormat == LOCAL_GL_RGBA || readGLFormat == LOCAL_GL_BGRA);
     MOZ_ASSERT(readType == LOCAL_GL_UNSIGNED_BYTE);
 
     // ReadPixels from the current FB into lockedBits.
     {
-      size_t alignment = 8;
-      if (dstStride % 4 == 0) alignment = 4;
-
       ScopedPackState scopedPackState(gl);
-      if (alignment != 4) {
-        gl->fPixelStorei(LOCAL_GL_PACK_ALIGNMENT, alignment);
-      }
-
+      bool handled = scopedPackState.SetForWidthAndStrideRGBA(width, dstStride);
+      MOZ_RELEASE_ASSERT(handled, "Unhandled stride");
       gl->raw_fReadPixels(0, 0, width, height, readGLFormat, readType,
                           dstBytes);
     }
   }
 
   const bool isReadRGBA = readGLFormat == LOCAL_GL_RGBA;
 
   if (isReadRGBA != isDstRGBA) {
--- a/gfx/layers/client/CanvasClient.cpp
+++ b/gfx/layers/client/CanvasClient.cpp
@@ -308,21 +308,22 @@ static already_AddRefed<TextureClient> T
     MOZ_ASSERT(succeeded, "texture should have locked");
 
     MappedTextureData mapped;
     texClient->BorrowMappedData(mapped);
 
     // ReadPixels from the current FB into mapped.data.
     auto width = src->mSize.width;
     auto height = src->mSize.height;
+    auto stride = mapped.stride;
 
     {
       ScopedPackState scopedPackState(gl);
-
-      MOZ_ASSERT(mapped.stride / 4 == mapped.size.width);
+      bool handled = scopedPackState.SetForWidthAndStrideRGBA(width, stride);
+      MOZ_RELEASE_ASSERT(handled, "Unhandled stride");
       gl->raw_fReadPixels(0, 0, width, height, readFormat, readType,
                           mapped.data);
     }
 
     // RB_SWAPPED doesn't work with D3D11. (bug 1051010)
     // RB_SWAPPED doesn't work with Basic. (bug ???????)
     bool layersNeedsManualSwap = layersBackend == LayersBackend::LAYERS_BASIC ||
                                  layersBackend == LayersBackend::LAYERS_D3D11;