Title: [229316] trunk/Source
Revision
229316
Author
zandober...@gmail.com
Date
2018-03-06 03:35:22 -0800 (Tue, 06 Mar 2018)

Log Message

[CoordGraphics] Clean up CoordinatedImageBacking
https://bugs.webkit.org/show_bug.cgi?id=183332

Reviewed by Carlos Garcia Campos.

Source/WebCore:

Clean up the CoordinatedImageBacking class. Prefer reference values in
class functions, methods and member variables, where possible. Move
member variables into a more sensible order. Initialize a few member
variables at the place of declaration.

Drop releaseSurfaceIfNeeded() and updateVisibilityIfNeeded() methods,
integrating them into the update() method, which was the only place
where they were called from.

We don't have to keep a reference to the buffer object, since we're
not using it internally after it's been passed to the client's
updateImageBacking() implementation.

No new tests -- no change in behavior.

* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::syncImageBacking):
(WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):
* platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:
(WebCore::CoordinatedImageBacking::getCoordinatedImageBackingID):
(WebCore::CoordinatedImageBacking::CoordinatedImageBacking):
(WebCore::CoordinatedImageBacking::addHost):
(WebCore::CoordinatedImageBacking::removeHost):
(WebCore::CoordinatedImageBacking::update):
(WebCore::CoordinatedImageBacking::clearContentsTimerFired):
(WebCore::CoordinatedImageBacking::create): Deleted.
(WebCore::CoordinatedImageBacking::markDirty): Deleted.
(WebCore::CoordinatedImageBacking::releaseSurfaceIfNeeded): Deleted.
(WebCore::CoordinatedImageBacking::updateVisibilityIfNeeded): Deleted.
* platform/graphics/texmap/coordinated/CoordinatedImageBacking.h:

Source/WebKit:

* WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
(WebKit::CompositingCoordinator::createImageBackingIfNeeded):
Adjust call to CoordinatedImageBacking::getCoordinatedImageBackingID().

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (229315 => 229316)


--- trunk/Source/WebCore/ChangeLog	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebCore/ChangeLog	2018-03-06 11:35:22 UTC (rev 229316)
@@ -1,5 +1,43 @@
 2018-03-06  Zan Dobersek  <zdober...@igalia.com>
 
+        [CoordGraphics] Clean up CoordinatedImageBacking
+        https://bugs.webkit.org/show_bug.cgi?id=183332
+
+        Reviewed by Carlos Garcia Campos.
+
+        Clean up the CoordinatedImageBacking class. Prefer reference values in
+        class functions, methods and member variables, where possible. Move
+        member variables into a more sensible order. Initialize a few member
+        variables at the place of declaration.
+
+        Drop releaseSurfaceIfNeeded() and updateVisibilityIfNeeded() methods,
+        integrating them into the update() method, which was the only place
+        where they were called from.
+
+        We don't have to keep a reference to the buffer object, since we're
+        not using it internally after it's been passed to the client's
+        updateImageBacking() implementation.
+
+        No new tests -- no change in behavior.
+
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
+        (WebCore::CoordinatedGraphicsLayer::syncImageBacking):
+        (WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):
+        * platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:
+        (WebCore::CoordinatedImageBacking::getCoordinatedImageBackingID):
+        (WebCore::CoordinatedImageBacking::CoordinatedImageBacking):
+        (WebCore::CoordinatedImageBacking::addHost):
+        (WebCore::CoordinatedImageBacking::removeHost):
+        (WebCore::CoordinatedImageBacking::update):
+        (WebCore::CoordinatedImageBacking::clearContentsTimerFired):
+        (WebCore::CoordinatedImageBacking::create): Deleted.
+        (WebCore::CoordinatedImageBacking::markDirty): Deleted.
+        (WebCore::CoordinatedImageBacking::releaseSurfaceIfNeeded): Deleted.
+        (WebCore::CoordinatedImageBacking::updateVisibilityIfNeeded): Deleted.
+        * platform/graphics/texmap/coordinated/CoordinatedImageBacking.h:
+
+2018-03-06  Zan Dobersek  <zdober...@igalia.com>
+
         GraphicsLayerTextureMapper: remove the setAnimations() method
         https://bugs.webkit.org/show_bug.cgi?id=183358
 

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp (229315 => 229316)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp	2018-03-06 11:35:22 UTC (rev 229316)
@@ -628,13 +628,13 @@
         ASSERT(!shouldHaveBackingStore());
         ASSERT(m_compositedImage);
 
-        bool imageInstanceReplaced = m_coordinatedImageBacking && (m_coordinatedImageBacking->id() != CoordinatedImageBacking::getCoordinatedImageBackingID(m_compositedImage.get()));
+        bool imageInstanceReplaced = m_coordinatedImageBacking && (m_coordinatedImageBacking->id() != CoordinatedImageBacking::getCoordinatedImageBackingID(*m_compositedImage));
         if (imageInstanceReplaced)
             releaseImageBackingIfNeeded();
 
         if (!m_coordinatedImageBacking) {
             m_coordinatedImageBacking = m_coordinator->createImageBackingIfNeeded(*m_compositedImage);
-            m_coordinatedImageBacking->addHost(this);
+            m_coordinatedImageBacking->addHost(*this);
             m_layerState.imageID = m_coordinatedImageBacking->id();
         }
 
@@ -790,7 +790,7 @@
         return;
 
     ASSERT(m_coordinator);
-    m_coordinatedImageBacking->removeHost(this);
+    m_coordinatedImageBacking->removeHost(*this);
     m_coordinatedImageBacking = nullptr;
     m_layerState.imageID = InvalidCoordinatedImageBackingID;
     m_layerState.imageChanged = true;

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp (229315 => 229316)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp	2018-03-06 11:35:22 UTC (rev 229316)
@@ -35,130 +35,96 @@
 
 namespace WebCore {
 
-CoordinatedImageBackingID CoordinatedImageBacking::getCoordinatedImageBackingID(Image* image)
+CoordinatedImageBackingID CoordinatedImageBacking::getCoordinatedImageBackingID(Image& image)
 {
     // CoordinatedImageBacking keeps a RefPtr<Image> member, so the same Image pointer can not refer two different instances until CoordinatedImageBacking releases the member.
-    return reinterpret_cast<CoordinatedImageBackingID>(image);
+    return reinterpret_cast<CoordinatedImageBackingID>(&image);
 }
 
-Ref<CoordinatedImageBacking> CoordinatedImageBacking::create(Client& client, Ref<Image>&& image)
-{
-    return adoptRef(*new CoordinatedImageBacking(client, WTFMove(image)));
-}
-
 CoordinatedImageBacking::CoordinatedImageBacking(Client& client, Ref<Image>&& image)
-    : m_client(&client)
+    : m_client(client)
+    , m_id(getCoordinatedImageBackingID(image))
     , m_image(WTFMove(image))
-    , m_id(getCoordinatedImageBackingID(m_image.get()))
     , m_clearContentsTimer(*this, &CoordinatedImageBacking::clearContentsTimerFired)
-    , m_isDirty(false)
-    , m_isVisible(false)
 {
-    // FIXME: We would need to decode a small image directly into a GraphicsSurface.
-    // http://webkit.org/b/101426
-
-    m_client->createImageBacking(id());
+    m_client.createImageBacking(m_id);
 }
 
 CoordinatedImageBacking::~CoordinatedImageBacking() = default;
 
-void CoordinatedImageBacking::addHost(Host* host)
+void CoordinatedImageBacking::addHost(Host& host)
 {
-    ASSERT(!m_hosts.contains(host));
-    m_hosts.append(host);
+    ASSERT(!m_hosts.contains(&host));
+    m_hosts.add(&host);
 }
 
-void CoordinatedImageBacking::removeHost(Host* host)
+void CoordinatedImageBacking::removeHost(Host& host)
 {
-    size_t position = m_hosts.find(host);
-    ASSERT(position != notFound);
-    m_hosts.remove(position);
+    m_hosts.remove(&host);
 
     if (m_hosts.isEmpty())
-        m_client->removeImageBacking(id());
+        m_client.removeImageBacking(m_id);
 }
 
-void CoordinatedImageBacking::markDirty()
-{
-    m_isDirty = true;
-}
+static const Seconds clearContentsTimerInterval { 3_s };
 
 void CoordinatedImageBacking::update()
 {
-    releaseSurfaceIfNeeded();
+    bool previousIsVisible = m_isVisible;
+    m_isVisible = std::any_of(m_hosts.begin(), m_hosts.end(),
+        [](auto* host)
+        {
+            return host->imageBackingVisible();
+        });
 
-    bool changedToVisible;
-    updateVisibilityIfNeeded(changedToVisible);
-    if (!m_isVisible)
+    if (!m_isVisible) {
+        if (previousIsVisible) {
+            ASSERT(!m_clearContentsTimer.isActive());
+            m_clearContentsTimer.startOneShot(clearContentsTimerInterval);
+        }
         return;
+    }
 
+    bool changedToVisible = !previousIsVisible;
+    if (m_clearContentsTimer.isActive()) {
+        m_clearContentsTimer.stop();
+        // We don't want to update the texture if we didn't remove the texture.
+        changedToVisible = false;
+    }
+
+    auto nativeImagePtr = m_image->nativeImageForCurrentFrame();
     if (!changedToVisible) {
         if (!m_isDirty)
             return;
 
-        if (m_nativeImagePtr == m_image->nativeImageForCurrentFrame()) {
+        if (m_nativeImagePtr == nativeImagePtr) {
             m_isDirty = false;
             return;
         }
     }
 
-    m_buffer = Nicosia::Buffer::create(IntSize(m_image->size()), !m_image->currentFrameKnownToBeOpaque() ? Nicosia::Buffer::SupportsAlpha : Nicosia::Buffer::NoFlags);
-    ASSERT(m_buffer);
+    m_nativeImagePtr = WTFMove(nativeImagePtr);
 
-    Nicosia::PaintingContext::paint(*m_buffer,
+    auto buffer = Nicosia::Buffer::create(IntSize(m_image->size()), !m_image->currentFrameKnownToBeOpaque() ? Nicosia::Buffer::SupportsAlpha : Nicosia::Buffer::NoFlags);
+    Nicosia::PaintingContext::paint(buffer,
         [this](GraphicsContext& context)
         {
             IntRect rect(IntPoint::zero(), IntSize(m_image->size()));
             context.save();
             context.clip(rect);
-            context.drawImage(*m_image, rect, rect);
+            context.drawImage(m_image, rect, rect);
             context.restore();
         });
 
-    m_nativeImagePtr = m_image->nativeImageForCurrentFrame();
-
-    m_client->updateImageBacking(id(), m_buffer.copyRef());
+    m_client.updateImageBacking(m_id, WTFMove(buffer));
     m_isDirty = false;
 }
 
-void CoordinatedImageBacking::releaseSurfaceIfNeeded()
-{
-    m_buffer = nullptr;
-}
-
-static const Seconds clearContentsTimerInterval { 3_s };
-
-void CoordinatedImageBacking::updateVisibilityIfNeeded(bool& changedToVisible)
-{
-    bool previousIsVisible = m_isVisible;
-
-    m_isVisible = false;
-    for (auto& host : m_hosts) {
-        if (host->imageBackingVisible()) {
-            m_isVisible = true;
-            break;
-        }
-    }
-
-    bool changedToInvisible = previousIsVisible && !m_isVisible;
-    if (changedToInvisible) {
-        ASSERT(!m_clearContentsTimer.isActive());
-        m_clearContentsTimer.startOneShot(clearContentsTimerInterval);
-    }
-
-    changedToVisible = !previousIsVisible && m_isVisible;
-
-    if (m_isVisible && m_clearContentsTimer.isActive()) {
-        m_clearContentsTimer.stop();
-        // We don't want to update the texture if we didn't remove the texture.
-        changedToVisible = false;
-    }
-}
-
 void CoordinatedImageBacking::clearContentsTimerFired()
 {
-    m_client->clearImageBackingContents(id());
+    m_client.clearImageBackingContents(m_id);
 }
 
 } // namespace WebCore
+
 #endif

Modified: trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.h (229315 => 229316)


--- trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.h	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.h	2018-03-06 11:35:22 UTC (rev 229316)
@@ -23,10 +23,8 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#pragma once
 
-#ifndef CoordinatedImageBacking_h
-#define CoordinatedImageBacking_h
-
 #if USE(COORDINATED_GRAPHICS)
 
 #include "CoordinatedGraphicsState.h"
@@ -56,17 +54,20 @@
         virtual bool imageBackingVisible() = 0;
     };
 
-    static Ref<CoordinatedImageBacking> create(Client&, Ref<Image>&&);
+    static Ref<CoordinatedImageBacking> create(Client& client, Ref<Image>&& image)
+    {
+        return adoptRef(*new CoordinatedImageBacking(client, WTFMove(image)));
+    }
     virtual ~CoordinatedImageBacking();
 
-    static CoordinatedImageBackingID getCoordinatedImageBackingID(Image*);
+    static CoordinatedImageBackingID getCoordinatedImageBackingID(Image&);
     CoordinatedImageBackingID id() const { return m_id; }
 
-    void addHost(Host*);
-    void removeHost(Host*);
+    void addHost(Host&);
+    void removeHost(Host&);
 
     // When a new image is updated or an animated gif is progressed, CoordinatedGraphicsLayer calls markDirty().
-    void markDirty();
+    void markDirty() { m_isDirty = true; }
 
     // Create, remove or update its backing.
     void update();
@@ -74,26 +75,21 @@
 private:
     CoordinatedImageBacking(Client&, Ref<Image>&&);
 
-    void releaseSurfaceIfNeeded();
-    void updateVisibilityIfNeeded(bool& changedToVisible);
     void clearContentsTimerFired();
 
-    Client* m_client;
-    RefPtr<Image> m_image;
+    Client& m_client;
+    HashSet<Host*> m_hosts;
+
+    CoordinatedImageBackingID m_id;
+    Ref<Image> m_image;
     NativeImagePtr m_nativeImagePtr;
-    CoordinatedImageBackingID m_id;
-    Vector<Host*> m_hosts;
 
-    RefPtr<Nicosia::Buffer> m_buffer;
-
     Timer m_clearContentsTimer;
 
-    bool m_isDirty;
-    bool m_isVisible;
-
+    bool m_isDirty { false };
+    bool m_isVisible { false };
 };
 
 } // namespace WebCore
+
 #endif // USE(COORDINATED_GRAPHICS)
-
-#endif // CoordinatedImageBacking_h

Modified: trunk/Source/WebKit/ChangeLog (229315 => 229316)


--- trunk/Source/WebKit/ChangeLog	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebKit/ChangeLog	2018-03-06 11:35:22 UTC (rev 229316)
@@ -1,5 +1,16 @@
 2018-03-06  Zan Dobersek  <zdober...@igalia.com>
 
+        [CoordGraphics] Clean up CoordinatedImageBacking
+        https://bugs.webkit.org/show_bug.cgi?id=183332
+
+        Reviewed by Carlos Garcia Campos.
+
+        * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
+        (WebKit::CompositingCoordinator::createImageBackingIfNeeded):
+        Adjust call to CoordinatedImageBacking::getCoordinatedImageBackingID().
+
+2018-03-06  Zan Dobersek  <zdober...@igalia.com>
+
         CoordinatedGraphicsScene: properly limit data specific to state commit operation
         https://bugs.webkit.org/show_bug.cgi?id=183326
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp (229315 => 229316)


--- trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp	2018-03-06 11:34:19 UTC (rev 229315)
+++ trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp	2018-03-06 11:35:22 UTC (rev 229316)
@@ -213,7 +213,7 @@
 
 Ref<CoordinatedImageBacking> CompositingCoordinator::createImageBackingIfNeeded(Image& image)
 {
-    CoordinatedImageBackingID imageID = CoordinatedImageBacking::getCoordinatedImageBackingID(&image);
+    CoordinatedImageBackingID imageID = CoordinatedImageBacking::getCoordinatedImageBackingID(image);
     auto addResult = m_imageBackings.ensure(imageID, [this, &image] {
         return CoordinatedImageBacking::create(*this, image);
     });
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to