More clean up per review request

libraries/gpu-gl/src/gpu/gl/GLTexture.cpp

@@ -673,3 +673,24 @@ void GLVariableAllocationSupport::executeNextBuffer(const TexturePointer& curren
     }
 }
 #endif
+
+void GLVariableAllocationSupport::incrementPopulatedSize(Size delta) const {
+    _populatedSize += delta;
+    // Keep the 2 code paths to be able to debug
+    if (_size < _populatedSize) {
+        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
+    } else  {
+        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
+    }
+}
+void GLVariableAllocationSupport::decrementPopulatedSize(Size delta) const {
+    _populatedSize -= delta;
+    // Keep the 2 code paths to be able to debug
+    if (_size < _populatedSize) {
+        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
+    } else  {
+        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
+    }
+}
+
+

libraries/gpu-gl/src/gpu/gl/GLTexture.h

@@ -127,6 +127,14 @@ protected:
     virtual void promote() = 0;
     virtual void demote() = 0;
 
+    // THe amount of memory currently allocated
+    Size _size { 0 };
+
+    // The amount of memory currnently populated
+    void incrementPopulatedSize(Size delta) const;
+    void decrementPopulatedSize(Size delta) const;
+    mutable Size _populatedSize { 0 };
+
     // The allocated mip level, relative to the number of mips in the gpu::Texture object 
     // The relationship between a given glMip to the original gpu::Texture mip is always 
     // glMip + _allocatedMip
@@ -174,8 +182,11 @@ public:
 protected:
     virtual Size size() const = 0;
     virtual void generateMips() const = 0;
+    virtual void syncSampler() const = 0;
+
     virtual Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const = 0;
     virtual Size copyMipFaceFromTexture(uint16_t sourceMip, uint16_t targetMip, uint8_t face) const final;
+    virtual void copyTextureMipsInGPUMem(GLuint srcId, GLuint destId, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) {} // Only relevant for Variable Allocation textures
 
     GLTexture(const std::weak_ptr<gl::GLBackend>& backend, const Texture& texture, GLuint id);
 };
@@ -188,6 +199,7 @@ public:
 protected:
     GLExternalTexture(const std::weak_ptr<gl::GLBackend>& backend, const Texture& texture, GLuint id);
     void generateMips() const override {}
+    void syncSampler() const override {}
     Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override { return 0;}
 
     Size size() const override { return 0; }

libraries/gpu-gl/src/gpu/gl41/GL41Backend.h

@@ -55,7 +55,7 @@ public:
         GL41Texture(const std::weak_ptr<GLBackend>& backend, const Texture& texture);
         void generateMips() const override;
         Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override;
-        virtual void syncSampler() const;
+        void syncSampler() const override;
 
         void withPreservedTexture(std::function<void()> f) const;
     };
@@ -114,11 +114,9 @@ public:
         Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override;
         Size copyMipsFromTexture();
 
+        void copyTextureMipsInGPUMem(GLuint srcId, GLuint destId, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) override;
+
         Size size() const override { return _size; }
-        Size _size { 0 };
-        void incrementPopulatedSize(Size delta) const;
-        void decrementPopulatedSize(Size delta) const;
-        mutable Size _populatedSize { 0 };
     };
 
     class GL41ResourceTexture : public GL41VariableAllocationTexture {

libraries/gpu-gl/src/gpu/gl41/GL41BackendTexture.cpp

@@ -288,25 +288,6 @@ GL41VariableAllocationTexture::~GL41VariableAllocationTexture() {
     Backend::textureResourcePopulatedGPUMemSize.update(_populatedSize, 0);
 }
 
-void GL41VariableAllocationTexture::incrementPopulatedSize(Size delta) const {
-    _populatedSize += delta;
-    // Keep the 2 code paths to be able to debug
-    if (_size < _populatedSize) {
-        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
-    } else  {
-        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
-    }
-}
-void GL41VariableAllocationTexture::decrementPopulatedSize(Size delta) const {
-    _populatedSize -= delta;
-    // Keep the 2 code paths to be able to debug
-    if (_size < _populatedSize) {
-        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
-    } else  {
-        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
-    }
-}
-
 void GL41VariableAllocationTexture::allocateStorage(uint16 allocatedMip) {
     _allocatedMip = allocatedMip;
 
@@ -479,6 +460,18 @@ void copyCompressedTexGPUMem(const gpu::Texture& texture, GLenum texTarget, GLui
     glDeleteBuffers(1, &pbo);
 }
 
+void GL41VariableAllocationTexture::copyTextureMipsInGPUMem(GLuint srcId, GLuint destId, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) {
+    uint16_t numMips = _gpuObject.getNumMips();
+    withPreservedTexture([&] {
+        if (_texelFormat.isCompressed()) {
+            copyCompressedTexGPUMem(_gpuObject, _target, srcId, destId, numMips, srcMipOffset, destMipOffset, populatedMips);
+        } else {
+            copyUncompressedTexGPUMem(_gpuObject, _target, srcId, destId, numMips, srcMipOffset, destMipOffset, populatedMips);
+        }
+        syncSampler();
+    });
+}
+
 void GL41VariableAllocationTexture::promote() {
     PROFILE_RANGE(render_gpu_gl, __FUNCTION__);
     Q_ASSERT(_allocatedMip > 0);
@@ -497,21 +490,18 @@ void GL41VariableAllocationTexture::promote() {
     allocateStorage(targetAllocatedMip);
 
     // copy pre-existing mips
-    uint16_t numMips = _gpuObject.getNumMips();
+    copyTextureMipsInGPUMem(oldId, _id, oldAllocatedMip, _allocatedMip, _populatedMip);
-    withPreservedTexture([&] {
-        if (_texelFormat.isCompressed()) {
-            copyCompressedTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
-        } else {
-            copyUncompressedTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
-        }
-        syncSampler();
-    });
 
     // destroy the old texture
     glDeleteTextures(1, &oldId);
+
+    // Update sampler
+    syncSampler();
+
     // update the memory usage
     Backend::textureResourceGPUMemSize.update(oldSize, 0);
     // no change to Backend::textureResourcePopulatedGPUMemSize
+
     populateTransferQueue();
 }
 
@@ -528,20 +518,16 @@ void GL41VariableAllocationTexture::demote() {
     allocateStorage(_allocatedMip + 1);
     _populatedMip = std::max(_populatedMip, _allocatedMip);
 
-    // copy pre-existing mips
-    uint16_t numMips = _gpuObject.getNumMips();
-    withPreservedTexture([&] {
-        if (_texelFormat.isCompressed()) {
-            copyCompressedTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
-        } else {
-            copyUncompressedTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
-        }
-        syncSampler();
-    });
 
+    // copy pre-existing mips
+    copyTextureMipsInGPUMem(oldId, _id, oldAllocatedMip, _allocatedMip, _populatedMip);
 
     // destroy the old texture
     glDeleteTextures(1, &oldId);
+
+    // Update sampler
+    syncSampler();
+
     // update the memory usage
     Backend::textureResourceGPUMemSize.update(oldSize, 0);
     // Demoting unpopulate the memory delta

libraries/gpu-gl/src/gpu/gl45/GL45Backend.h

@@ -51,7 +51,7 @@ public:
         GL45Texture(const std::weak_ptr<GLBackend>& backend, const Texture& texture);
         void generateMips() const override;
         Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override;
-        virtual void syncSampler() const;
+        void syncSampler() const override;
     };
 
     //
@@ -98,17 +98,16 @@ public:
         friend class GL45Backend;
         using PromoteLambda = std::function<void()>;
 
-        // Overight copy to inject counter for populated amount
-        Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override;
 
     protected:
         GL45VariableAllocationTexture(const std::weak_ptr<GLBackend>& backend, const Texture& texture);
         ~GL45VariableAllocationTexture();
+
         Size size() const override { return _size; }
-        Size _size { 0 };
+
-        void incrementPopulatedSize(Size delta) const;
+        Size copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const override;
-        void decrementPopulatedSize(Size delta) const;
+        void copyTextureMipsInGPUMem(GLuint srcId, GLuint destId, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) override;
-        mutable Size _populatedSize { 0 };
+
     };
 
     class GL45ResourceTexture : public GL45VariableAllocationTexture {
@@ -122,6 +121,7 @@ public:
         void demote() override;
         void populateTransferQueue() override;
         
+
         void allocateStorage(uint16 mip);
         Size copyMipsFromTexture();
     };

libraries/gpu-gl/src/gpu/gl45/GL45BackendVariableTexture.cpp

@@ -39,24 +39,6 @@ GL45VariableAllocationTexture::~GL45VariableAllocationTexture() {
     Backend::textureResourceGPUMemSize.update(_size, 0);
     Backend::textureResourcePopulatedGPUMemSize.update(_populatedSize, 0);
 }
-void GL45VariableAllocationTexture::incrementPopulatedSize(Size delta) const {
-    _populatedSize += delta;
-    // Keep the 2 code paths to be able to debug
-    if (_size < _populatedSize) {
-        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
-    } else  {
-        Backend::textureResourcePopulatedGPUMemSize.update(0, delta);
-    }
-}
-void GL45VariableAllocationTexture::decrementPopulatedSize(Size delta) const {
-    _populatedSize -= delta;
-    // Keep the 2 code paths to be able to debug
-    if (_size < _populatedSize) {
-        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
-    } else  {
-        Backend::textureResourcePopulatedGPUMemSize.update(delta, 0);
-    }
-}
 
 Size GL45VariableAllocationTexture::copyMipFaceLinesFromTexture(uint16_t mip, uint8_t face, const uvec3& size, uint32_t yOffset, GLenum internalFormat, GLenum format, GLenum type, Size sourceSize, const void* sourcePointer) const {
     Size amountCopied = 0;
@@ -65,6 +47,30 @@ Size GL45VariableAllocationTexture::copyMipFaceLinesFromTexture(uint16_t mip, ui
     return amountCopied;
 }
 
+
+void copyTexGPUMem(const gpu::Texture& texture, GLenum texTarget, GLuint srcId, GLuint destId, uint16_t numMips, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) {
+    for (uint16_t mip = populatedMips; mip < numMips; ++mip) {
+        auto mipDimensions = texture.evalMipDimensions(mip);
+        uint16_t targetMip = mip - destMipOffset;
+        uint16_t sourceMip = mip - srcMipOffset;
+        auto faces = GLTexture::getFaceCount(texTarget);
+        for (uint8_t face = 0; face < faces; ++face) {
+            glCopyImageSubData(
+                srcId, texTarget, sourceMip, 0, 0, face,
+                destId, texTarget, targetMip, 0, 0, face,
+                mipDimensions.x, mipDimensions.y, 1
+                );
+            (void)CHECK_GL_ERROR();
+        }
+    }
+}
+
+void GL45VariableAllocationTexture::copyTextureMipsInGPUMem(GLuint srcId, GLuint destId, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) {
+    uint16_t numMips = _gpuObject.getNumMips();
+    copyTexGPUMem(_gpuObject, _target, srcId, destId, numMips, srcMipOffset, destMipOffset, populatedMips);
+}
+
+
 // Managed size resource textures
 using GL45ResourceTexture = GL45Backend::GL45ResourceTexture;
 
@@ -124,24 +130,6 @@ void GL45ResourceTexture::syncSampler() const {
     glTextureParameteri(_id, GL_TEXTURE_BASE_LEVEL, _populatedMip - _allocatedMip);
 }
 
-
-void copyTexGPUMem(const gpu::Texture& texture, GLenum texTarget, GLuint srcId, GLuint destId, uint16_t numMips, uint16_t srcMipOffset, uint16_t destMipOffset, uint16_t populatedMips) {
-    for (uint16_t mip = populatedMips; mip < numMips; ++mip) {
-        auto mipDimensions = texture.evalMipDimensions(mip);
-        uint16_t targetMip = mip - destMipOffset;
-        uint16_t sourceMip = mip - srcMipOffset;
-        auto faces = GLTexture::getFaceCount(texTarget);
-        for (uint8_t face = 0; face < faces; ++face) {
-            glCopyImageSubData(
-                srcId, texTarget, sourceMip, 0, 0, face,
-                destId, texTarget, targetMip, 0, 0, face,
-                mipDimensions.x, mipDimensions.y, 1
-                );
-            (void)CHECK_GL_ERROR();
-        }
-    }
-}
-
 void GL45ResourceTexture::promote() {
     PROFILE_RANGE(render_gpu_gl, __FUNCTION__);
     Q_ASSERT(_allocatedMip > 0);
@@ -160,15 +148,18 @@ void GL45ResourceTexture::promote() {
     allocateStorage(targetAllocatedMip);
     
     // copy pre-existing mips
-    uint16_t numMips = _gpuObject.getNumMips();
+    copyTextureMipsInGPUMem(oldId, _id, oldAllocatedMip, _allocatedMip, _populatedMip);
-    copyTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
 
     // destroy the old texture
     glDeleteTextures(1, &oldId);
+
+    // Update sampler
+    syncSampler();
+
     // update the memory usage
     Backend::textureResourceGPUMemSize.update(oldSize, 0);
     // no change to Backend::textureResourcePopulatedGPUMemSize
-    syncSampler();
+
     populateTransferQueue();
 }
 
@@ -186,11 +177,14 @@ void GL45ResourceTexture::demote() {
     _populatedMip = std::max(_populatedMip, _allocatedMip);
 
     // copy pre-existing mips
-    uint16_t numMips = _gpuObject.getNumMips();
+    copyTextureMipsInGPUMem(oldId, _id, oldAllocatedMip, _allocatedMip, _populatedMip);
-    copyTexGPUMem(_gpuObject, _target, oldId, _id, numMips, oldAllocatedMip, _allocatedMip, _populatedMip);
 
     // destroy the old texture
     glDeleteTextures(1, &oldId);
+
+    // Update sampler
+    syncSampler();
+
     // update the memory usage
     Backend::textureResourceGPUMemSize.update(oldSize, 0);
      // Demoting unpopulate the memory delta
@@ -202,11 +196,10 @@ void GL45ResourceTexture::demote() {
         }
        decrementPopulatedSize(amountUnpopulated);
     }
-    syncSampler();
+
     populateTransferQueue();
 }
 
-
 void GL45ResourceTexture::populateTransferQueue() {
     PROFILE_RANGE(render_gpu_gl, __FUNCTION__);
     if (_populatedMip <= _allocatedMip) {