From b1411a66d24e74cca2c3335edf215451023c73da Mon Sep 17 00:00:00 2001 From: Jamie Nicol Date: Fri, 21 Oct 2016 13:27:41 +0100 Subject: [PATCH] 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 | 25 +++------ gfx/gl/GLUploadHelpers.cpp | 109 +++++++++++++------------------------ gfx/gl/GLUploadHelpers.h | 68 +++++++++++------------ gfx/gl/TextureImageEGL.cpp | 4 +- 4 files changed, 81 insertions(+), 125 deletions(-) diff --git a/gfx/gl/GLTextureImage.cpp b/gfx/gl/GLTextureImage.cpp index 4ffd14235a28..c91d558af441 100644 --- a/gfx/gl/GLTextureImage.cpp +++ b/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)); diff --git a/gfx/gl/GLUploadHelpers.cpp b/gfx/gl/GLUploadHelpers.cpp index c4246b141736..29cad46acf8f 100644 --- a/gfx/gl/GLUploadHelpers.cpp +++ b/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 diff --git a/gfx/gl/GLUploadHelpers.h b/gfx/gl/GLUploadHelpers.h index ebcc096f9b03..866d44adbed0 100644 --- a/gfx/gl/GLUploadHelpers.h +++ b/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); diff --git a/gfx/gl/TextureImageEGL.cpp b/gfx/gl/TextureImageEGL.cpp index 33a0ced5a5ab..87a547c26925 100644 --- a/gfx/gl/TextureImageEGL.cpp +++ b/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); }