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

--HG--
extra : rebase_source : f4415fcec15bd2a7ca9300ffcf26439b96a72438

gfx/gl/GLTextureImage.cpp

@@ -131,26 +131,25 @@ BasicTextureImage::BindTexture(GLenum aTextureUnit)
 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);
-    } else {
+    if (mTextureState == Valid) {
         region = aRegion;
+    } else {
+        region = nsIntRegion(IntRect(0, 0, mSize.width, mSize.height));
     }
-
-    size_t uploadSize;
     bool needInit = mTextureState == Created;
+    size_t uploadSize;
+
     mTextureFormat =
         UploadSurfaceToTexture(mGLContext,
                                aSurf,
                                region,
                                mTexture,
+                               mSize,
                                &uploadSize,
                                needInit,
-                               bounds.TopLeft() + IntPoint(aFrom.x, aFrom.y),
-                               false);
+                               aFrom);
+
     if (uploadSize > 0) {
         UpdateUploadSize(uploadSize);
     }
@@ -295,13 +294,7 @@ TiledTextureImage::DirectUpdate(gfx::DataSourceSurface* aSurf, const nsIntRegion
         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));

gfx/gl/GLUploadHelpers.cpp

@@ -356,43 +356,16 @@ UploadImageDataToTexture(GLContext* gl,
                          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;
@@ -473,26 +446,39 @@ UploadImageDataToTexture(GLContext* gl,
             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,
@@ -505,25 +491,6 @@ UploadImageDataToTexture(GLContext* gl,
                                 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;
         }
     }
 
@@ -534,22 +501,24 @@ 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

gfx/gl/GLUploadHelpers.h

@@ -22,62 +22,56 @@ 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);
 

gfx/gl/TextureImageEGL.cpp

@@ -115,10 +115,10 @@ TextureImageEGL::DirectUpdate(gfx::DataSourceSurface* aSurf, const nsIntRegion&
                              aSurf,
                              region,
                              mTexture,
+                             mSize,
                              &uploadSize,
                              needInit,
-                             bounds.TopLeft() + gfx::IntPoint(aFrom.x, aFrom.y),
-                             false);
+                             aFrom);
     if (uploadSize > 0) {
         UpdateUploadSize(uploadSize);
     }