Bug 1311642 - Give GLUploadHelpers some love. r=nical
authorJamie Nicol <jnicol@mozilla.com>
Fri, 21 Oct 2016 13:27:41 +0100
changeset 319448 4a4b3074ade68a04ea020a319229dcf59422df30
parent 319447 b0b2881014728cae83a25992df40c0db38437b90
child 319449 262687aa0bf271c01809f54c6cccd71e3ae052ae
push id83172
push userphilringnalda@gmail.com
push dateWed, 26 Oct 2016 05:07:25 +0000
treeherdermozilla-inbound@c141993d03ee [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1311642
milestone52.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 1311642 - Give GLUploadHelpers some love. r=nical GLUploadHelpers was trying to be too flexible, so remove some unused options it provided. Require that the full surface and texture size is passed in rather than only the updated subregion. This prevents textures being initialized to an incorrect size when partial texture uploads are disallowed. MozReview-Commit-ID: 288ERE9ten5
gfx/gl/GLTextureImage.cpp
gfx/gl/GLUploadHelpers.cpp
gfx/gl/GLUploadHelpers.h
gfx/gl/TextureImageEGL.cpp
--- a/gfx/gl/GLTextureImage.cpp
+++ b/gfx/gl/GLTextureImage.cpp
@@ -126,36 +126,35 @@ BasicTextureImage::BindTexture(GLenum aT
     mGLContext->fActiveTexture(aTextureUnit);
     mGLContext->fBindTexture(LOCAL_GL_TEXTURE_2D, mTexture);
     mGLContext->fActiveTexture(LOCAL_GL_TEXTURE0);
 }
 
 bool
 BasicTextureImage::DirectUpdate(gfx::DataSourceSurface* aSurf, const nsIntRegion& aRegion, const gfx::IntPoint& aFrom /* = gfx::IntPoint(0, 0) */)
 {
-    IntRect bounds = aRegion.GetBounds();
     nsIntRegion region;
-    if (mTextureState != Valid) {
-        bounds = IntRect(0, 0, mSize.width, mSize.height);
-        region = nsIntRegion(bounds);
+    if (mTextureState == Valid) {
+        region = aRegion;
     } else {
-        region = aRegion;
+        region = nsIntRegion(IntRect(0, 0, mSize.width, mSize.height));
     }
+    bool needInit = mTextureState == Created;
+    size_t uploadSize;
 
-    size_t uploadSize;
-    bool needInit = mTextureState == Created;
     mTextureFormat =
         UploadSurfaceToTexture(mGLContext,
                                aSurf,
                                region,
                                mTexture,
+                               mSize,
                                &uploadSize,
                                needInit,
-                               bounds.TopLeft() + IntPoint(aFrom.x, aFrom.y),
-                               false);
+                               aFrom);
+
     if (uploadSize > 0) {
         UpdateUploadSize(uploadSize);
     }
     mTextureState = Valid;
     return true;
 }
 
 void
@@ -290,23 +289,17 @@ TiledTextureImage::DirectUpdate(gfx::Dat
         int yPos = tileRect.y;
 
         nsIntRegion tileRegion;
         tileRegion.And(region, tileRect); // intersect with tile
 
         if (tileRegion.IsEmpty())
             continue;
 
-        if (CanUploadSubTextures(mGL)) {
-          tileRegion.MoveBy(-xPos, -yPos); // translate into tile local space
-        } else {
-          // If sub-textures are unsupported, expand to tile boundaries
-          tileRect.x = tileRect.y = 0;
-          tileRegion = nsIntRegion(tileRect);
-        }
+        tileRegion.MoveBy(-xPos, -yPos); // translate into tile local space
 
         result &= mImages[mCurrentImage]->
           DirectUpdate(aSurf, tileRegion, aFrom + gfx::IntPoint(xPos, yPos));
 
         if (mCurrentImage == mImages.Length() - 1) {
             // We know we're done, but we still need to ensure that the callback
             // gets called (e.g. to update the uploaded region).
             NextTile();
--- a/gfx/gl/GLUploadHelpers.cpp
+++ b/gfx/gl/GLUploadHelpers.cpp
@@ -351,53 +351,26 @@ TexImage2DHelper(GLContext* gl,
 }
 
 SurfaceFormat
 UploadImageDataToTexture(GLContext* gl,
                          unsigned char* aData,
                          int32_t aStride,
                          SurfaceFormat aFormat,
                          const nsIntRegion& aDstRegion,
-                         GLuint& aTexture,
+                         GLuint aTexture,
+                         const gfx::IntSize& aSize,
                          size_t* aOutUploadSize,
                          bool aNeedInit,
-                         bool aPixelBuffer,
                          GLenum aTextureUnit,
                          GLenum aTextureTarget)
 {
-    bool textureInited = aNeedInit ? false : true;
     gl->MakeCurrent();
     gl->fActiveTexture(aTextureUnit);
-
-    if (!aTexture) {
-        gl->fGenTextures(1, &aTexture);
-        gl->fBindTexture(aTextureTarget, aTexture);
-        gl->fTexParameteri(aTextureTarget,
-                           LOCAL_GL_TEXTURE_MIN_FILTER,
-                           LOCAL_GL_LINEAR);
-        gl->fTexParameteri(aTextureTarget,
-                           LOCAL_GL_TEXTURE_MAG_FILTER,
-                           LOCAL_GL_LINEAR);
-        gl->fTexParameteri(aTextureTarget,
-                           LOCAL_GL_TEXTURE_WRAP_S,
-                           LOCAL_GL_CLAMP_TO_EDGE);
-        gl->fTexParameteri(aTextureTarget,
-                           LOCAL_GL_TEXTURE_WRAP_T,
-                           LOCAL_GL_CLAMP_TO_EDGE);
-        textureInited = false;
-    } else {
-        gl->fBindTexture(aTextureTarget, aTexture);
-    }
-
-    nsIntRegion paintRegion;
-    if (!textureInited) {
-        paintRegion = nsIntRegion(aDstRegion.GetBounds());
-    } else {
-        paintRegion = aDstRegion;
-    }
+    gl->fBindTexture(aTextureTarget, aTexture);
 
     GLenum format = 0;
     GLenum internalFormat = 0;
     GLenum type = 0;
     int32_t pixelSize = BytesPerPixel(aFormat);
     SurfaceFormat surfaceFormat = gfx::SurfaceFormat::UNKNOWN;
 
     MOZ_ASSERT(gl->GetPreferredARGB32Format() == LOCAL_GL_BGRA ||
@@ -468,93 +441,89 @@ UploadImageDataToTexture(GLContext* gl,
             type = LOCAL_GL_UNSIGNED_BYTE;
             // We don't have a specific luminance shader
             surfaceFormat = SurfaceFormat::A8;
             break;
         default:
             NS_ASSERTION(false, "Unhandled image surface format!");
     }
 
-
-    // Top left point of the region's bounding rectangle.
-    IntPoint topLeft = paintRegion.GetBounds().TopLeft();
-
     if (aOutUploadSize) {
         *aOutUploadSize = 0;
     }
 
-    for (auto iter = paintRegion.RectIter(); !iter.Done(); iter.Next()) {
-        const IntRect& rect = iter.Get();
-        // The inital data pointer is at the top left point of the region's
-        // bounding rectangle. We need to find the offset of this rect
-        // within the region and adjust the data pointer accordingly.
-        unsigned char* rectData =
-            aData + DataOffset(rect.TopLeft() - topLeft, aStride, aFormat);
+    if (aNeedInit || !CanUploadSubTextures(gl)) {
+        // If the texture needs initialized, or we are unable to
+        // upload sub textures, then initialize and upload the entire
+        // texture.
+        TexImage2DHelper(gl,
+                         aTextureTarget,
+                         0,
+                         internalFormat,
+                         aSize.width,
+                         aSize.height,
+                         aStride,
+                         pixelSize,
+                         0,
+                         format,
+                         type,
+                         aData);
 
-        NS_ASSERTION(textureInited || (rect.x == 0 && rect.y == 0),
-                     "Must be uploading to the origin when we don't have an existing texture");
+        if (aOutUploadSize && aNeedInit) {
+            uint32_t texelSize = GetBytesPerTexel(internalFormat, type);
+            size_t numTexels = size_t(aSize.width) * size_t(aSize.height);
+            *aOutUploadSize += texelSize * numTexels;
+        }
+    } else {
+        // Upload each rect in the region to the texture
+        for (auto iter = aDstRegion.RectIter(); !iter.Done(); iter.Next()) {
+            const IntRect& rect = iter.Get();
+            const unsigned char* rectData =
+                aData + DataOffset(rect.TopLeft(), aStride, aFormat);
 
-        if (textureInited && CanUploadSubTextures(gl)) {
             TexSubImage2DHelper(gl,
                                 aTextureTarget,
                                 0,
                                 rect.x,
                                 rect.y,
                                 rect.width,
                                 rect.height,
                                 aStride,
                                 pixelSize,
                                 format,
                                 type,
                                 rectData);
-        } else {
-            TexImage2DHelper(gl,
-                             aTextureTarget,
-                             0,
-                             internalFormat,
-                             rect.width,
-                             rect.height,
-                             aStride,
-                             pixelSize,
-                             0,
-                             format,
-                             type,
-                             rectData);
-        }
-
-        if (aOutUploadSize && !textureInited) {
-            uint32_t texelSize = GetBytesPerTexel(internalFormat, type);
-            size_t numTexels = size_t(rect.width) * size_t(rect.height);
-            *aOutUploadSize += texelSize * numTexels;
         }
     }
 
     return surfaceFormat;
 }
 
 SurfaceFormat
 UploadSurfaceToTexture(GLContext* gl,
                        DataSourceSurface* aSurface,
                        const nsIntRegion& aDstRegion,
-                       GLuint& aTexture,
+                       GLuint aTexture,
+                       const gfx::IntSize& aSize,
                        size_t* aOutUploadSize,
                        bool aNeedInit,
                        const gfx::IntPoint& aSrcPoint,
-                       bool aPixelBuffer,
                        GLenum aTextureUnit,
                        GLenum aTextureTarget)
 {
-    unsigned char* data = aPixelBuffer ? nullptr : aSurface->GetData();
+
     int32_t stride = aSurface->Stride();
     SurfaceFormat format = aSurface->GetFormat();
-    data += DataOffset(aSrcPoint, stride, format);
+    unsigned char* data = aSurface->GetData() +
+        DataOffset(aSrcPoint, stride, format);
+
     return UploadImageDataToTexture(gl, data, stride, format,
-                                    aDstRegion, aTexture, aOutUploadSize,
-                                    aNeedInit, aPixelBuffer, aTextureUnit,
-                                    aTextureTarget);
+                                    aDstRegion, aTexture, aSize,
+                                    aOutUploadSize, aNeedInit,
+                                    aTextureUnit, aTextureTarget);
 }
 
 bool
 CanUploadNonPowerOfTwo(GLContext* gl)
 {
     if (!gl->WorkAroundDriverBugs())
         return true;
 
--- a/gfx/gl/GLUploadHelpers.h
+++ b/gfx/gl/GLUploadHelpers.h
@@ -17,72 +17,66 @@ namespace gfx {
 class DataSourceSurface;
 } // namespace gfx
 
 namespace gl {
 
 class GLContext;
 
 /**
-  * Creates a RGB/RGBA texture (or uses one provided) and uploads the surface
-  * contents to it within aSrcRect.
-  *
-  * aSrcRect.x/y will be uploaded to 0/0 in the texture, and the size
-  * of the texture with be aSrcRect.width/height.
-  *
-  * If an existing texture is passed through aTexture, it is assumed it
-  * has already been initialised with glTexImage2D (or this function),
-  * and that its size is equal to or greater than aSrcRect + aDstPoint.
-  * You can alternatively set the overwrite flag to true and have a new
-  * texture memory block allocated.
-  *
-  * The aDstPoint parameter is ignored if no texture was provided
-  * or aOverwrite is true.
-  *
-  * \param aData Image data to upload.
-  * \param aDstRegion Region of texture to upload to.
-  * \param aTexture Texture to use, or 0 to have one created for you.
-  * \param aOutUploadSize if set, the number of bytes the texture requires will be returned here
-  * \param aOverwrite Over an existing texture with a new one.
-  * \param aSrcPoint Offset into aSrc where the region's bound's
-  *  TopLeft() sits.
-  * \param aPixelBuffer Pass true to upload texture data with an
-  *  offset from the base data (generally for pixel buffer objects),
-  *  otherwise textures are upload with an absolute pointer to the data.
-  * \param aTextureUnit, the texture unit used temporarily to upload the
-  *  surface. This testure may be overridden, clients should not rely on
-  *  the contents of this texture after this call or even on this
-  *  texture unit being active.
-  * \return Surface format of this texture.
-  */
+ * Uploads image data to an OpenGL texture, initializing the texture
+ * first if necessary.
+ *
+ * \param gl The GL Context to use.
+ * \param aData Start of image data of surface to upload.
+ *              Corresponds to the first pixel of the texture.
+ * \param aStride The image data's stride.
+ * \param aFormat The image data's format.
+ * \param aDstRegion Region of the texture to upload.
+ * \param aTexture The OpenGL texture to use. Must refer to a valid texture.
+ * \param aSize The full size of the texture.
+ * \param aOutUploadSize If set, the number of bytes the texture requires will
+ *                       be returned here.
+ * \param aNeedInit Indicates whether a new texture must be allocated.
+ * \param aTextureUnit The texture unit used temporarily to upload the surface.
+ *                     This may be overridden, so clients should not rely on
+ *                     the aTexture being bound to aTextureUnit after this call,
+ *                     or even on aTextureUnit being active.
+ * \param aTextureTarget The texture target to use.
+ * \return Surface format of this texture.
+ */
 gfx::SurfaceFormat
 UploadImageDataToTexture(GLContext* gl,
                          unsigned char* aData,
                          int32_t aStride,
                          gfx::SurfaceFormat aFormat,
                          const nsIntRegion& aDstRegion,
-                         GLuint& aTexture,
+                         GLuint aTexture,
+                         const gfx::IntSize& aSize,
                          size_t* aOutUploadSize = nullptr,
                          bool aNeedInit = false,
-                         bool aPixelBuffer = false,
                          GLenum aTextureUnit = LOCAL_GL_TEXTURE0,
                          GLenum aTextureTarget = LOCAL_GL_TEXTURE_2D);
 
 /**
-  * Convenience wrapper around UploadImageDataToTexture for gfx::DataSourceSurface's.
-  */
+ * Convenience wrapper around UploadImageDataToTexture for
+ * gfx::DataSourceSurface's.
+ *
+ * \param aSurface The surface from which to upload image data.
+ * \param aSrcPoint Offset into aSurface where this texture's data begins.
+ */
 gfx::SurfaceFormat
 UploadSurfaceToTexture(GLContext* gl,
                        gfx::DataSourceSurface* aSurface,
                        const nsIntRegion& aDstRegion,
-                       GLuint& aTexture,
+                       GLuint aTexture,
+                       const gfx::IntSize& aSize,
                        size_t* aOutUploadSize = nullptr,
                        bool aNeedInit = false,
                        const gfx::IntPoint& aSrcPoint = gfx::IntPoint(0, 0),
-                       bool aPixelBuffer = false,
                        GLenum aTextureUnit = LOCAL_GL_TEXTURE0,
                        GLenum aTextureTarget = LOCAL_GL_TEXTURE_2D);
 
 bool CanUploadSubTextures(GLContext* gl);
 bool CanUploadNonPowerOfTwo(GLContext* gl);
 
 } // namespace gl
 } // namespace mozilla
--- a/gfx/gl/TextureImageEGL.cpp
+++ b/gfx/gl/TextureImageEGL.cpp
@@ -110,20 +110,20 @@ TextureImageEGL::DirectUpdate(gfx::DataS
 
     bool needInit = mTextureState == Created;
     size_t uploadSize = 0;
     mTextureFormat =
       UploadSurfaceToTexture(mGLContext,
                              aSurf,
                              region,
                              mTexture,
+                             mSize,
                              &uploadSize,
                              needInit,
-                             bounds.TopLeft() + gfx::IntPoint(aFrom.x, aFrom.y),
-                             false);
+                             aFrom);
     if (uploadSize > 0) {
         UpdateUploadSize(uploadSize);
     }
 
     mTextureState = Valid;
     return true;
 }