[TexMap] Clean up TextureMapperGLData, TextureMapperGLData::SharedGLData
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2016 07:36:17 +0000 (07:36 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 May 2016 07:36:17 +0000 (07:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157303

Reviewed by Carlos Garcia Campos.

Torch the TextureMapperGLData and TextureMapperGLData::SharedGLData
classes into a better form.

SharedGLData is now private to the TextureMapperGLData class, but
lists that class as a friend. This enables moving the getShaderProgram()
method to TextureMapperGLData and removes the need to expose the
SharedGLData publicly. SharedGLData also doesn't have to keep a
reference to the GraphicsContext3D object.

TextureMapperGLData now default-initializes the publicly accessible
member variables while making some other members private, not exposing
anything that isn't necessary.

Usual whitespace cleanup included. More elegant HashMap::ensure()
insertions are used where they don't obfuscate object creation.

No new tests -- no change in behavior.

* platform/graphics/texmap/TextureMapperGL.cpp:
(WebCore::TextureMapperGLData::SharedGLData::currentSharedGLData):
(WebCore::TextureMapperGLData::SharedGLData::~SharedGLData):
(WebCore::TextureMapperGLData::SharedGLData::contextDataMap):
(WebCore::TextureMapperGLData::SharedGLData::SharedGLData):
(WebCore::TextureMapperGLData::TextureMapperGLData):
(WebCore::TextureMapperGLData::~TextureMapperGLData):
(WebCore::TextureMapperGLData::initializeStencil):
(WebCore::TextureMapperGLData::getStaticVBO):
(WebCore::TextureMapperGLData::getShaderProgram):
(WebCore::TextureMapperGL::TextureMapperGL):
(WebCore::TextureMapperGL::drawBorder):
(WebCore::TextureMapperGL::drawTexture):
(WebCore::TextureMapperGL::drawSolidColor):
(WebCore::TextureMapperGL::drawFiltered):
(WebCore::TextureMapperGL::beginClip):
(WebCore::TextureMapperGLData::SharedGLData::glContextDataMap): Deleted.
(WebCore::TextureMapperGLData::SharedGLData::getShaderProgram): Deleted.
(WebCore::TextureMapperGLData::sharedGLData): Deleted.
* platform/graphics/texmap/TextureMapperGL.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@200407 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp
Source/WebCore/platform/graphics/texmap/TextureMapperGL.h

index b8daba3..54dfebc 100644 (file)
@@ -1,3 +1,49 @@
+2016-05-04  Zan Dobersek  <zdobersek@igalia.com>
+
+        [TexMap] Clean up TextureMapperGLData, TextureMapperGLData::SharedGLData
+        https://bugs.webkit.org/show_bug.cgi?id=157303
+
+        Reviewed by Carlos Garcia Campos.
+
+        Torch the TextureMapperGLData and TextureMapperGLData::SharedGLData
+        classes into a better form.
+
+        SharedGLData is now private to the TextureMapperGLData class, but
+        lists that class as a friend. This enables moving the getShaderProgram()
+        method to TextureMapperGLData and removes the need to expose the
+        SharedGLData publicly. SharedGLData also doesn't have to keep a
+        reference to the GraphicsContext3D object.
+
+        TextureMapperGLData now default-initializes the publicly accessible
+        member variables while making some other members private, not exposing
+        anything that isn't necessary.
+
+        Usual whitespace cleanup included. More elegant HashMap::ensure()
+        insertions are used where they don't obfuscate object creation.
+
+        No new tests -- no change in behavior.
+
+        * platform/graphics/texmap/TextureMapperGL.cpp:
+        (WebCore::TextureMapperGLData::SharedGLData::currentSharedGLData):
+        (WebCore::TextureMapperGLData::SharedGLData::~SharedGLData):
+        (WebCore::TextureMapperGLData::SharedGLData::contextDataMap):
+        (WebCore::TextureMapperGLData::SharedGLData::SharedGLData):
+        (WebCore::TextureMapperGLData::TextureMapperGLData):
+        (WebCore::TextureMapperGLData::~TextureMapperGLData):
+        (WebCore::TextureMapperGLData::initializeStencil):
+        (WebCore::TextureMapperGLData::getStaticVBO):
+        (WebCore::TextureMapperGLData::getShaderProgram):
+        (WebCore::TextureMapperGL::TextureMapperGL):
+        (WebCore::TextureMapperGL::drawBorder):
+        (WebCore::TextureMapperGL::drawTexture):
+        (WebCore::TextureMapperGL::drawSolidColor):
+        (WebCore::TextureMapperGL::drawFiltered):
+        (WebCore::TextureMapperGL::beginClip):
+        (WebCore::TextureMapperGLData::SharedGLData::glContextDataMap): Deleted.
+        (WebCore::TextureMapperGLData::SharedGLData::getShaderProgram): Deleted.
+        (WebCore::TextureMapperGLData::sharedGLData): Deleted.
+        * platform/graphics/texmap/TextureMapperGL.h:
+
 2016-05-03  Chris Dumez  <cdumez@apple.com>
 
         Optimize [StrictTypeChecking] on IDL attributes
index f2e6949..13bb88b 100644 (file)
@@ -35,6 +35,7 @@
 #include "TextureMapperShaderProgram.h"
 #include "Timer.h"
 #include <wtf/HashMap.h>
+#include <wtf/NeverDestroyed.h>
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
 #include <wtf/TemporaryChange.h>
 #endif
 
 namespace WebCore {
-struct TextureMapperGLData {
+
+class TextureMapperGLData {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    struct SharedGLData : public RefCounted<SharedGLData> {
+    explicit TextureMapperGLData(GraphicsContext3D&);
+    ~TextureMapperGLData();
 
-        typedef HashMap<PlatformGraphicsContext3D, SharedGLData*> GLContextDataMap;
-        static GLContextDataMap& glContextDataMap()
-        {
-            static GLContextDataMap map;
-            return map;
-        }
+    void initializeStencil();
+    Platform3DObject getStaticVBO(GC3Denum target, GC3Dsizeiptr, const void* data);
+    Ref<TextureMapperShaderProgram> getShaderProgram(TextureMapperShaderProgram::Options);
 
-        static PassRefPtr<SharedGLData> currentSharedGLData(GraphicsContext3D* context)
-        {
-            GLContextDataMap::iterator it = glContextDataMap().find(context->platformGraphicsContext3D());
-            if (it != glContextDataMap().end())
-                return it->value;
+    TransformationMatrix projectionMatrix;
+    TextureMapper::PaintFlags PaintFlags { 0 };
+    GC3Dint previousProgram { 0 };
+    GC3Dint targetFrameBuffer { 0 };
+    bool didModifyStencil { false };
+    GC3Dint previousScissorState { 0 };
+    GC3Dint previousDepthState { 0 };
+    GC3Dint viewport[4] { 0, };
+    GC3Dint previousScissor[4] { 0, };
+    RefPtr<BitmapTexture> currentSurface;
+    const BitmapTextureGL::FilterInfo* filterInfo { nullptr };
 
-            return adoptRef(new SharedGLData(context));
+private:
+    class SharedGLData : public RefCounted<SharedGLData> {
+    public:
+        static Ref<SharedGLData> currentSharedGLData(GraphicsContext3D& context)
+        {
+            RefPtr<SharedGLData> data;
+            auto addResult = contextDataMap().add(context.platformGraphicsContext3D(), nullptr);
+            if (addResult.isNewEntry) {
+                data = adoptRef(new SharedGLData(context));
+                addResult.iterator->value = data.get();
+            } else
+                data = addResult.iterator->value;
+
+            return *data;
         }
 
-        PassRefPtr<TextureMapperShaderProgram> getShaderProgram(TextureMapperShaderProgram::Options options)
+        ~SharedGLData()
         {
-            HashMap<TextureMapperShaderProgram::Options, RefPtr<TextureMapperShaderProgram> >::AddResult result = m_programs.add(options, nullptr);
-            if (result.isNewEntry)
-                result.iterator->value = TextureMapperShaderProgram::create(m_context, options);
-
-            return result.iterator->value;
+            RELEASE_ASSERT(contextDataMap().contains(this));
+            contextDataMap().removeIf([this] (GLContextDataMap::KeyValuePairType& entry) { return entry.value == this; });
         }
 
-        HashMap<TextureMapperShaderProgram::Options, RefPtr<TextureMapperShaderProgram> > m_programs;
-        RefPtr<GraphicsContext3D> m_context;
+    private:
+        friend class TextureMapperGLData;
 
-        explicit SharedGLData(GraphicsContext3D* context)
-            : m_context(context)
+        using GLContextDataMap = HashMap<PlatformGraphicsContext3D, SharedGLData*>;
+        static GLContextDataMap& contextDataMap()
         {
-            glContextDataMap().add(context->platformGraphicsContext3D(), this);
+            static NeverDestroyed<GLContextDataMap> map;
+            return map;
         }
 
-        ~SharedGLData()
+        explicit SharedGLData(GraphicsContext3D& context)
         {
-            for (auto it = glContextDataMap().begin(), end = glContextDataMap().end(); it != end; ++it) {
-                if (it->value == this) {
-                    glContextDataMap().remove(it);
-                    return;
-                }
-            }
-
-            ASSERT_NOT_REACHED();
+            contextDataMap().add(context.platformGraphicsContext3D(), this);
         }
-    };
-
-    SharedGLData& sharedGLData() const
-    {
-        return *sharedData;
-    }
 
-    void initializeStencil();
-
-    explicit TextureMapperGLData(GraphicsContext3D* context)
-        : context(context)
-        , PaintFlags(0)
-        , previousProgram(0)
-        , targetFrameBuffer(0)
-        , didModifyStencil(false)
-        , previousScissorState(0)
-        , previousDepthState(0)
-        , sharedData(TextureMapperGLData::SharedGLData::currentSharedGLData(this->context))
-        , filterInfo(0)
-    { }
-
-    ~TextureMapperGLData();
-    Platform3DObject getStaticVBO(GC3Denum target, GC3Dsizeiptr, const void* data);
+        HashMap<TextureMapperShaderProgram::Options, RefPtr<TextureMapperShaderProgram>> m_programs;
+    };
 
-    GraphicsContext3D* context;
-    TransformationMatrix projectionMatrix;
-    TextureMapper::PaintFlags PaintFlags;
-    GC3Dint previousProgram;
-    GC3Dint targetFrameBuffer;
-    bool didModifyStencil;
-    GC3Dint previousScissorState;
-    GC3Dint previousDepthState;
-    GC3Dint viewport[4];
-    GC3Dint previousScissor[4];
-    RefPtr<SharedGLData> sharedData;
-    RefPtr<BitmapTexture> currentSurface;
-    HashMap<const void*, Platform3DObject> vbos;
-    const BitmapTextureGL::FilterInfo* filterInfo;
+    GraphicsContext3D& m_context;
+    Ref<SharedGLData> m_sharedGLData;
+    HashMap<const void*, Platform3DObject> m_vbos;
 };
 
-Platform3DObject TextureMapperGLData::getStaticVBO(GC3Denum target, GC3Dsizeiptr size, const void* data)
+TextureMapperGLData::TextureMapperGLData(GraphicsContext3D& context)
+    : m_context(context)
+    , m_sharedGLData(SharedGLData::currentSharedGLData(m_context))
 {
-    HashMap<const void*, Platform3DObject>::AddResult result = vbos.add(data, 0);
-    if (result.isNewEntry) {
-        Platform3DObject vbo = context->createBuffer();
-        context->bindBuffer(target, vbo);
-        context->bufferData(target, size, data, GraphicsContext3D::STATIC_DRAW);
-        result.iterator->value = vbo;
-    }
-
-    return result.iterator->value;
 }
 
 TextureMapperGLData::~TextureMapperGLData()
 {
-    for (auto& entry : vbos)
-        context->deleteBuffer(entry.value);
+    for (auto& entry : m_vbos)
+        m_context.deleteBuffer(entry.value);
 }
 
 void TextureMapperGLData::initializeStencil()
@@ -166,16 +138,37 @@ void TextureMapperGLData::initializeStencil()
     if (didModifyStencil)
         return;
 
-    context->clearStencil(0);
-    context->clear(GraphicsContext3D::STENCIL_BUFFER_BIT);
+    m_context.clearStencil(0);
+    m_context.clear(GraphicsContext3D::STENCIL_BUFFER_BIT);
     didModifyStencil = true;
 }
 
+Platform3DObject TextureMapperGLData::getStaticVBO(GC3Denum target, GC3Dsizeiptr size, const void* data)
+{
+    auto addResult = m_vbos.ensure(data,
+        [this, target, size, data] {
+            Platform3DObject vbo = m_context.createBuffer();
+            m_context.bindBuffer(target, vbo);
+            m_context.bufferData(target, size, data, GraphicsContext3D::STATIC_DRAW);
+            return vbo;
+        });
+    return addResult.iterator->value;
+}
+
+Ref<TextureMapperShaderProgram> TextureMapperGLData::getShaderProgram(TextureMapperShaderProgram::Options options)
+{
+    auto addResult = m_sharedGLData->m_programs.ensure(options,
+        [this, options] { return TextureMapperShaderProgram::create(Ref<GraphicsContext3D>(m_context), options); });
+    return *addResult.iterator->value;
+}
+
 TextureMapperGL::TextureMapperGL()
     : m_enableEdgeDistanceAntialiasing(false)
 {
     m_context3D = GraphicsContext3D::createForCurrentGLContext();
-    m_data = new TextureMapperGLData(m_context3D.get());
+    ASSERT(m_context3D);
+
+    m_data = new TextureMapperGLData(*m_context3D);
 #if USE(TEXTURE_MAPPER_GL)
     m_texturePool = std::make_unique<BitmapTexturePool>(m_context3D.copyRef());
 #endif
@@ -229,7 +222,7 @@ void TextureMapperGL::drawBorder(const Color& color, float width, const FloatRec
     if (clipStack().isCurrentScissorBoxEmpty())
         return;
 
-    RefPtr<TextureMapperShaderProgram> program = data().sharedGLData().getShaderProgram(TextureMapperShaderProgram::SolidColor);
+    RefPtr<TextureMapperShaderProgram> program = data().getShaderProgram(TextureMapperShaderProgram::SolidColor);
     m_context3D->useProgram(program->programID());
 
     float r, g, b, a;
@@ -456,8 +449,7 @@ void TextureMapperGL::drawTexture(Platform3DObject texture, Flags flags, const I
     if (useAntialiasing || opacity < 1)
         flags |= ShouldBlend;
 
-    RefPtr<TextureMapperShaderProgram> program;
-    program = data().sharedGLData().getShaderProgram(options);
+    RefPtr<TextureMapperShaderProgram> program = data().getShaderProgram(options);
 
     if (filter)
         prepareFilterProgram(program.get(), *filter.get(), data().filterInfo->pass, textureSize, filterContentTextureID);
@@ -474,7 +466,7 @@ void TextureMapperGL::drawSolidColor(const FloatRect& rect, const Transformation
         flags |= ShouldBlend | ShouldAntialias;
     }
 
-    RefPtr<TextureMapperShaderProgram> program = data().sharedGLData().getShaderProgram(options);
+    RefPtr<TextureMapperShaderProgram> program = data().getShaderProgram(options);
     m_context3D->useProgram(program->programID());
 
     float r, g, b, a;
@@ -591,7 +583,7 @@ void TextureMapperGL::drawFiltered(const BitmapTexture& sampler, const BitmapTex
 {
     // For standard filters, we always draw the whole texture without transformations.
     TextureMapperShaderProgram::Options options = optionsForFilterType(filter.type(), pass);
-    RefPtr<TextureMapperShaderProgram> program = data().sharedGLData().getShaderProgram(options);
+    RefPtr<TextureMapperShaderProgram> program = data().getShaderProgram(options);
     ASSERT(program);
 
     prepareFilterProgram(program.get(), filter, pass, sampler.contentSize(), contentTexture ? static_cast<const BitmapTextureGL*>(contentTexture)->id() : 0);
@@ -669,7 +661,7 @@ void TextureMapperGL::beginClip(const TransformationMatrix& modelViewMatrix, con
 
     data().initializeStencil();
 
-    RefPtr<TextureMapperShaderProgram> program = data().sharedGLData().getShaderProgram(TextureMapperShaderProgram::SolidColor);
+    RefPtr<TextureMapperShaderProgram> program = data().getShaderProgram(TextureMapperShaderProgram::SolidColor);
 
     m_context3D->useProgram(program->programID());
     m_context3D->enableVertexAttribArray(program->vertexLocation());
index 2f47799..0d7a131 100644 (file)
@@ -33,7 +33,7 @@
 
 namespace WebCore {
 
-struct TextureMapperGLData;
+class TextureMapperGLData;
 class TextureMapperShaderProgram;
 class FilterOperation;