Title: [251874] trunk/Source/WebCore
Revision
251874
Author
commit-qu...@webkit.org
Date
2019-10-31 13:41:56 -0700 (Thu, 31 Oct 2019)

Log Message

Move ImageBuffer-related functionality from HTMLCanvasElement to CanvasBase
https://bugs.webkit.org/show_bug.cgi?id=182503

Patch by Zan Dobersek  <zdober...@igalia.com>  and  Chris Lord <cl...@igalia.com> on 2019-10-31
Reviewed by Ryosuke Niwa.

Move the ImageBuffer member variable, other related member variables and
majority of methods that operate on these from the HTMLCanvasElement
class up to the CanvasBase class. This will make it possible for the
OffscreenCanvas implementation to leverage the same code when using 2D
contexts for painting.

Most of the moved methods are public, while the setImageBuffer() method
is made protected so that it's still accessible from the inheriting
class.

Along with setImageBuffer(), the active pixel memory counter is moved
into the CanvasBase class. It's still using static storage, but is now
a member of the class with protected access. The storage has been made
atomic so as to allow its consistency to be maintained when accessed
from multiple threads (which may happen in the future).

The m_size member variable is also moved up into the CanvasBase class.
Constructor is changed so that the passed-in IntSize argument is used
to initialize it. Canvas implementations are still the owners of their
respective canvas contexts and are responsible for the destruction of
both the context and the ImageBuffer, in that order.

HTMLCanvasElement destructor still has to invoke
CanvasBase::notifyObserversCanvasDestroyed() since some CanvasObserver
derivatives perform virtual method calls on the CanvasBase object for
typecasting purposes in their canvasDestroyed() implementation.
Calling virtual methods on an object that's being destroyed is normally
discouraged and should be fixed separately, but it works as long as
invocations are done before the HTMLCanvasElement object is destroyed
completely (as has been the case so far).

CanvasRenderingContext2DBase is already changed in unwindStateStack()
to call CanvasBase::existingDrawingContext() and not downcast the
CanvasBase object to HTMLCanvasElement. This is done now due to
unwindStateStack() being called from the destructor, which is now
invoked from the CanvasBase destructor and not the HTMLCanvasElement
destructor, avoiding virtual method calls on the CanvasBase object as
it's being destroyed.

This patch doesn't address various methods using const qualifier and
then working around that by requiring member variables to be mutable.
This should be amended as much as possible in a separate patch.

No new tests -- no change in functionality, only refactoring.

* html/CanvasBase.cpp:
(WebCore::CanvasBase::CanvasBase):
(WebCore::CanvasBase::~CanvasBase):
(WebCore::CanvasBase::drawingContext const):
(WebCore::CanvasBase::existingDrawingContext const):
(WebCore::CanvasBase::buffer const):
(WebCore::CanvasBase::baseTransform const):
(WebCore::CanvasBase::makeRenderingResultsAvailable):
(WebCore::CanvasBase::memoryCost const):
(WebCore::CanvasBase::externalMemoryCost const):
(WebCore::CanvasBase::callTracingActive const):
(WebCore::CanvasBase::setImageBuffer const):
(WebCore::CanvasBase::activePixelMemory):
(WebCore::CanvasBase::resetGraphicsContextState const):
* html/CanvasBase.h:
(WebCore::CanvasBase::width const):
(WebCore::CanvasBase::height const):
(WebCore::CanvasBase::size const):
(WebCore::CanvasBase::setSize):
(WebCore::CanvasBase::hasCreatedImageBuffer const):
(WebCore::CanvasBase::createImageBuffer const):
* html/CustomPaintCanvas.cpp:
(WebCore::CustomPaintCanvas::CustomPaintCanvas):
(WebCore::CustomPaintCanvas::~CustomPaintCanvas):
* html/CustomPaintCanvas.h:
* html/HTMLCanvasElement.cpp:
(WebCore::HTMLCanvasElement::HTMLCanvasElement):
(WebCore::HTMLCanvasElement::~HTMLCanvasElement):
(WebCore::HTMLCanvasElement::setSize):
(WebCore::HTMLCanvasElement::createContext2d):
(WebCore::HTMLCanvasElement::reset):
(WebCore::HTMLCanvasElement::setSurfaceSize):
(WebCore::HTMLCanvasElement::toDataURL):
(WebCore::HTMLCanvasElement::toBlob):
(WebCore::HTMLCanvasElement::createImageBuffer const):
(WebCore::HTMLCanvasElement::setImageBufferAndMarkDirty):
* html/HTMLCanvasElement.h:
* html/OffscreenCanvas.cpp:
(WebCore::OffscreenCanvas::OffscreenCanvas):
(WebCore::OffscreenCanvas::~OffscreenCanvas):
(WebCore::OffscreenCanvas::setWidth):
(WebCore::OffscreenCanvas::setHeight):
(WebCore::OffscreenCanvas::transferToImageBitmap):
(WebCore::OffscreenCanvas::createImageBuffer const):
* html/OffscreenCanvas.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (251873 => 251874)


--- trunk/Source/WebCore/ChangeLog	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/ChangeLog	2019-10-31 20:41:56 UTC (rev 251874)
@@ -1,3 +1,101 @@
+2019-10-31  Zan Dobersek  <zdober...@igalia.com>  and  Chris Lord  <cl...@igalia.com>
+
+        Move ImageBuffer-related functionality from HTMLCanvasElement to CanvasBase
+        https://bugs.webkit.org/show_bug.cgi?id=182503
+
+        Reviewed by Ryosuke Niwa.
+
+        Move the ImageBuffer member variable, other related member variables and
+        majority of methods that operate on these from the HTMLCanvasElement
+        class up to the CanvasBase class. This will make it possible for the
+        OffscreenCanvas implementation to leverage the same code when using 2D
+        contexts for painting.
+
+        Most of the moved methods are public, while the setImageBuffer() method
+        is made protected so that it's still accessible from the inheriting
+        class.
+
+        Along with setImageBuffer(), the active pixel memory counter is moved
+        into the CanvasBase class. It's still using static storage, but is now
+        a member of the class with protected access. The storage has been made
+        atomic so as to allow its consistency to be maintained when accessed
+        from multiple threads (which may happen in the future).
+
+        The m_size member variable is also moved up into the CanvasBase class.
+        Constructor is changed so that the passed-in IntSize argument is used
+        to initialize it. Canvas implementations are still the owners of their
+        respective canvas contexts and are responsible for the destruction of
+        both the context and the ImageBuffer, in that order.
+
+        HTMLCanvasElement destructor still has to invoke
+        CanvasBase::notifyObserversCanvasDestroyed() since some CanvasObserver
+        derivatives perform virtual method calls on the CanvasBase object for
+        typecasting purposes in their canvasDestroyed() implementation.
+        Calling virtual methods on an object that's being destroyed is normally
+        discouraged and should be fixed separately, but it works as long as
+        invocations are done before the HTMLCanvasElement object is destroyed
+        completely (as has been the case so far).
+
+        CanvasRenderingContext2DBase is already changed in unwindStateStack()
+        to call CanvasBase::existingDrawingContext() and not downcast the
+        CanvasBase object to HTMLCanvasElement. This is done now due to
+        unwindStateStack() being called from the destructor, which is now
+        invoked from the CanvasBase destructor and not the HTMLCanvasElement
+        destructor, avoiding virtual method calls on the CanvasBase object as
+        it's being destroyed.
+
+        This patch doesn't address various methods using const qualifier and
+        then working around that by requiring member variables to be mutable.
+        This should be amended as much as possible in a separate patch.
+
+        No new tests -- no change in functionality, only refactoring.
+
+        * html/CanvasBase.cpp:
+        (WebCore::CanvasBase::CanvasBase):
+        (WebCore::CanvasBase::~CanvasBase):
+        (WebCore::CanvasBase::drawingContext const):
+        (WebCore::CanvasBase::existingDrawingContext const):
+        (WebCore::CanvasBase::buffer const):
+        (WebCore::CanvasBase::baseTransform const):
+        (WebCore::CanvasBase::makeRenderingResultsAvailable):
+        (WebCore::CanvasBase::memoryCost const):
+        (WebCore::CanvasBase::externalMemoryCost const):
+        (WebCore::CanvasBase::callTracingActive const):
+        (WebCore::CanvasBase::setImageBuffer const):
+        (WebCore::CanvasBase::activePixelMemory):
+        (WebCore::CanvasBase::resetGraphicsContextState const):
+        * html/CanvasBase.h:
+        (WebCore::CanvasBase::width const):
+        (WebCore::CanvasBase::height const):
+        (WebCore::CanvasBase::size const):
+        (WebCore::CanvasBase::setSize):
+        (WebCore::CanvasBase::hasCreatedImageBuffer const):
+        (WebCore::CanvasBase::createImageBuffer const):
+        * html/CustomPaintCanvas.cpp:
+        (WebCore::CustomPaintCanvas::CustomPaintCanvas):
+        (WebCore::CustomPaintCanvas::~CustomPaintCanvas):
+        * html/CustomPaintCanvas.h:
+        * html/HTMLCanvasElement.cpp:
+        (WebCore::HTMLCanvasElement::HTMLCanvasElement):
+        (WebCore::HTMLCanvasElement::~HTMLCanvasElement):
+        (WebCore::HTMLCanvasElement::setSize):
+        (WebCore::HTMLCanvasElement::createContext2d):
+        (WebCore::HTMLCanvasElement::reset):
+        (WebCore::HTMLCanvasElement::setSurfaceSize):
+        (WebCore::HTMLCanvasElement::toDataURL):
+        (WebCore::HTMLCanvasElement::toBlob):
+        (WebCore::HTMLCanvasElement::createImageBuffer const):
+        (WebCore::HTMLCanvasElement::setImageBufferAndMarkDirty):
+        * html/HTMLCanvasElement.h:
+        * html/OffscreenCanvas.cpp:
+        (WebCore::OffscreenCanvas::OffscreenCanvas):
+        (WebCore::OffscreenCanvas::~OffscreenCanvas):
+        (WebCore::OffscreenCanvas::setWidth):
+        (WebCore::OffscreenCanvas::setHeight):
+        (WebCore::OffscreenCanvas::transferToImageBitmap):
+        (WebCore::OffscreenCanvas::createImageBuffer const):
+        * html/OffscreenCanvas.h:
+
 2019-10-31  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: DOMDebugger: Node Removed breakpoints should fire whenever the node is removed from the main DOM tree, not just when it's removed from it's parent

Modified: trunk/Source/WebCore/html/CanvasBase.cpp (251873 => 251874)


--- trunk/Source/WebCore/html/CanvasBase.cpp	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/CanvasBase.cpp	2019-10-31 20:41:56 UTC (rev 251874)
@@ -31,26 +31,93 @@
 #include "Element.h"
 #include "FloatRect.h"
 #include "InspectorInstrumentation.h"
+#include <_javascript_Core/JSCInlines.h>
+#include <_javascript_Core/JSLock.h>
+#include <atomic>
 #include <wtf/Vector.h>
 
+static std::atomic<size_t> s_activePixelMemory { 0 };
+
 namespace WebCore {
 
-CanvasBase::CanvasBase()
+#if USE(CG)
+// FIXME: It seems strange that the default quality is not the one that is literally named "default".
+// Should fix names to make this easier to understand, or write an excellent comment here explaining why not.
+const InterpolationQuality defaultInterpolationQuality = InterpolationLow;
+#else
+const InterpolationQuality defaultInterpolationQuality = InterpolationDefault;
+#endif
+
+CanvasBase::CanvasBase(IntSize size)
+    : m_size(size)
 {
 }
 
 CanvasBase::~CanvasBase()
 {
-    ASSERT(!m_context); // Should have been set to null by base class.
     ASSERT(m_didNotifyObserversCanvasDestroyed);
     ASSERT(m_observers.isEmpty());
+    ASSERT(!m_imageBuffer);
 }
 
-CanvasRenderingContext* CanvasBase::renderingContext() const
+GraphicsContext* CanvasBase::drawingContext() const
 {
-    return m_context.get();
+    auto* context = renderingContext();
+    if (context && !context->is2d())
+        return nullptr;
+
+    return buffer() ? &m_imageBuffer->context() : nullptr;
 }
 
+GraphicsContext* CanvasBase::existingDrawingContext() const
+{
+    if (!hasCreatedImageBuffer())
+        return nullptr;
+
+    return drawingContext();
+}
+
+ImageBuffer* CanvasBase::buffer() const
+{
+    if (!hasCreatedImageBuffer())
+        createImageBuffer();
+    return m_imageBuffer.get();
+}
+
+AffineTransform CanvasBase::baseTransform() const
+{
+    ASSERT(hasCreatedImageBuffer());
+    return m_imageBuffer->baseTransform();
+}
+
+void CanvasBase::makeRenderingResultsAvailable()
+{
+    if (auto* context = renderingContext())
+        context->paintRenderingResultsToCanvas();
+}
+
+size_t CanvasBase::memoryCost() const
+{
+    // memoryCost() may be invoked concurrently from a GC thread, and we need to be careful
+    // about what data we access here and how. We need to hold a lock to prevent m_imageBuffer
+    // from being changed while we access it.
+    auto locker = holdLock(m_imageBufferAssignmentLock);
+    if (!m_imageBuffer)
+        return 0;
+    return m_imageBuffer->memoryCost();
+}
+
+size_t CanvasBase::externalMemoryCost() const
+{
+    // externalMemoryCost() may be invoked concurrently from a GC thread, and we need to be careful
+    // about what data we access here and how. We need to hold a lock to prevent m_imageBuffer
+    // from being changed while we access it.
+    auto locker = holdLock(m_imageBufferAssignmentLock);
+    if (!m_imageBuffer)
+        return 0;
+    return m_imageBuffer->externalMemoryCost();
+}
+
 void CanvasBase::addObserver(CanvasObserver& observer)
 {
     m_observers.add(&observer);
@@ -111,7 +178,53 @@
 
 bool CanvasBase::callTracingActive() const
 {
-    return m_context && m_context->callTracingActive();
+    auto* context = renderingContext();
+    return context && context->callTracingActive();
 }
 
+void CanvasBase::setImageBuffer(std::unique_ptr<ImageBuffer>&& buffer) const
+{
+    {
+        auto locker = holdLock(m_imageBufferAssignmentLock);
+        m_contextStateSaver = nullptr;
+        m_imageBuffer = WTFMove(buffer);
+    }
+
+    if (m_imageBuffer && m_size != m_imageBuffer->internalSize())
+        m_size = m_imageBuffer->internalSize();
+
+    size_t previousMemoryCost = m_imageBufferCost;
+    m_imageBufferCost = memoryCost();
+    s_activePixelMemory += m_imageBufferCost - previousMemoryCost;
+
+    auto* context = renderingContext();
+    if (context && m_imageBuffer && previousMemoryCost != m_imageBufferCost)
+        InspectorInstrumentation::didChangeCanvasMemory(*context);
+
+    if (!m_imageBuffer)
+        return;
+
+    m_imageBuffer->context().setShadowsIgnoreTransforms(true);
+    m_imageBuffer->context().setImageInterpolationQuality(defaultInterpolationQuality);
+    m_imageBuffer->context().setStrokeThickness(1);
+    m_contextStateSaver = makeUnique<GraphicsContextStateSaver>(m_imageBuffer->context());
+
+    JSC::JSLockHolder lock(scriptExecutionContext()->vm());
+    scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(m_imageBufferCost);
 }
+
+size_t CanvasBase::activePixelMemory()
+{
+    return s_activePixelMemory.load();
+}
+
+void CanvasBase::resetGraphicsContextState() const
+{
+    if (m_contextStateSaver) {
+        // Reset to the initial graphics context state.
+        m_contextStateSaver->restore();
+        m_contextStateSaver->save();
+    }
+}
+
+}

Modified: trunk/Source/WebCore/html/CanvasBase.h (251873 => 251874)


--- trunk/Source/WebCore/html/CanvasBase.h	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/CanvasBase.h	2019-10-31 20:41:56 UTC (rev 251874)
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include "IntSize.h"
 #include <wtf/HashSet.h>
 #include <wtf/TypeCasts.h>
 
@@ -35,9 +36,9 @@
 class CanvasRenderingContext;
 class Element;
 class GraphicsContext;
+class GraphicsContextStateSaver;
 class Image;
 class ImageBuffer;
-class IntSize;
 class FloatRect;
 class ScriptExecutionContext;
 class SecurityOrigin;
@@ -64,11 +65,19 @@
     virtual bool isOffscreenCanvas() const { return false; }
     virtual bool isCustomPaintCanvas() const { return false; }
 
-    virtual unsigned width() const = 0;
-    virtual unsigned height() const = 0;
-    virtual const IntSize& size() const  = 0;
-    virtual void setSize(const IntSize&) = 0;
+    unsigned width() const { return m_size.width(); }
+    unsigned height() const { return m_size.height(); }
+    const IntSize& size() const { return m_size; }
 
+    ImageBuffer* buffer() const;
+
+    virtual AffineTransform baseTransform() const;
+
+    void makeRenderingResultsAvailable();
+
+    size_t memoryCost() const;
+    size_t externalMemoryCost() const;
+
     void setOriginClean() { m_originClean = true; }
     void setOriginTainted() { m_originClean = false; }
     bool originClean() const { return m_originClean; }
@@ -76,7 +85,7 @@
     virtual SecurityOrigin* securityOrigin() const { return nullptr; }
     ScriptExecutionContext* scriptExecutionContext() const { return canvasBaseScriptExecutionContext();  }
 
-    CanvasRenderingContext* renderingContext() const;
+    virtual CanvasRenderingContext* renderingContext() const = 0;
 
     void addObserver(CanvasObserver&);
     void removeObserver(CanvasObserver&);
@@ -86,25 +95,36 @@
 
     HashSet<Element*> cssCanvasClients() const;
 
-    virtual GraphicsContext* drawingContext() const = 0;
-    virtual GraphicsContext* existingDrawingContext() const = 0;
+    virtual GraphicsContext* drawingContext() const;
+    virtual GraphicsContext* existingDrawingContext() const;
 
-    virtual void makeRenderingResultsAvailable() = 0;
     virtual void didDraw(const FloatRect&) = 0;
 
-    virtual AffineTransform baseTransform() const = 0;
     virtual Image* copiedImage() const = 0;
-
     bool callTracingActive() const;
 
 protected:
-    CanvasBase();
+    explicit CanvasBase(IntSize);
 
     virtual ScriptExecutionContext* canvasBaseScriptExecutionContext() const = 0;
 
-    std::unique_ptr<CanvasRenderingContext> m_context;
+    virtual void setSize(const IntSize& size) { m_size = size; }
 
+    void setImageBuffer(std::unique_ptr<ImageBuffer>&&) const;
+    virtual bool hasCreatedImageBuffer() const { return false; }
+    static size_t activePixelMemory();
+
+    void resetGraphicsContextState() const;
+
 private:
+    virtual void createImageBuffer() const { }
+
+    mutable IntSize m_size;
+    mutable Lock m_imageBufferAssignmentLock;
+    mutable std::unique_ptr<ImageBuffer> m_imageBuffer;
+    mutable size_t m_imageBufferCost { 0 };
+    mutable std::unique_ptr<GraphicsContextStateSaver> m_contextStateSaver;
+
     bool m_originClean { true };
 #ifndef NDEBUG
     bool m_didNotifyObserversCanvasDestroyed { false };

Modified: trunk/Source/WebCore/html/CustomPaintCanvas.cpp (251873 => 251874)


--- trunk/Source/WebCore/html/CustomPaintCanvas.cpp	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/CustomPaintCanvas.cpp	2019-10-31 20:41:56 UTC (rev 251874)
@@ -28,6 +28,7 @@
 
 #if ENABLE(CSS_PAINTING_API)
 
+#include "CanvasRenderingContext.h"
 #include "ImageBitmap.h"
 #include "PaintRenderingContext2D.h"
 
@@ -39,8 +40,8 @@
 }
 
 CustomPaintCanvas::CustomPaintCanvas(ScriptExecutionContext& context, unsigned width, unsigned height)
-    : ContextDestructionObserver(&context)
-    , m_size(width, height)
+    : CanvasBase(IntSize(width, height))
+    , ContextDestructionObserver(&context)
 {
 }
 
@@ -49,38 +50,9 @@
     notifyObserversCanvasDestroyed();
 
     m_context = nullptr; // Ensure this goes away before the ImageBuffer.
+    setImageBuffer(nullptr);
 }
 
-unsigned CustomPaintCanvas::width() const
-{
-    return m_size.width();
-}
-
-void CustomPaintCanvas::setWidth(unsigned newWidth)
-{
-    return m_size.setWidth(newWidth);
-}
-
-unsigned CustomPaintCanvas::height() const
-{
-    return m_size.height();
-}
-
-void CustomPaintCanvas::setHeight(unsigned newHeight)
-{
-    return m_size.setHeight(newHeight);
-}
-
-const IntSize& CustomPaintCanvas::size() const
-{
-    return m_size;
-}
-
-void CustomPaintCanvas::setSize(const IntSize& newSize)
-{
-    m_size = newSize;
-}
-
 ExceptionOr<RefPtr<PaintRenderingContext2D>> CustomPaintCanvas::getContext()
 {
     if (m_context)
@@ -144,11 +116,5 @@
     return drawingContext();
 }
 
-void CustomPaintCanvas::makeRenderingResultsAvailable()
-{
-    if (m_context)
-        m_context->paintRenderingResultsToCanvas();
 }
-
-}
 #endif

Modified: trunk/Source/WebCore/html/CustomPaintCanvas.h (251873 => 251874)


--- trunk/Source/WebCore/html/CustomPaintCanvas.h	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/CustomPaintCanvas.h	2019-10-31 20:41:56 UTC (rev 251874)
@@ -41,6 +41,7 @@
 
 namespace WebCore {
 
+class CanvasRenderingContext;
 class ImageBitmap;
 class PaintRenderingContext2D;
 
@@ -52,20 +53,12 @@
     virtual ~CustomPaintCanvas();
     bool isCustomPaintCanvas() const final { return true; }
 
-    unsigned width() const final;
-    void setWidth(unsigned);
-    unsigned height() const final;
-    void setHeight(unsigned);
-
-    const IntSize& size() const final;
-    void setSize(const IntSize&) final;
-
     ExceptionOr<RefPtr<PaintRenderingContext2D>> getContext();
 
+    CanvasRenderingContext* renderingContext() const final { return m_context.get(); }
     GraphicsContext* drawingContext() const final;
     GraphicsContext* existingDrawingContext() const final;
 
-    void makeRenderingResultsAvailable() final;
     void didDraw(const FloatRect&) final { }
 
     AffineTransform baseTransform() const final { ASSERT(m_destinationGraphicsContext && m_copiedBuffer); return m_copiedBuffer->baseTransform(); }
@@ -82,8 +75,8 @@
     void derefCanvasBase() final { deref(); }
     ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
 
+    std::unique_ptr<CanvasRenderingContext> m_context;
     mutable GraphicsContext* m_destinationGraphicsContext = nullptr;
-    mutable IntSize m_size;
     mutable std::unique_ptr<ImageBuffer> m_copiedBuffer;
     mutable RefPtr<Image> m_copiedImage;
 };

Modified: trunk/Source/WebCore/html/HTMLCanvasElement.cpp (251873 => 251874)


--- trunk/Source/WebCore/html/HTMLCanvasElement.cpp	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/HTMLCanvasElement.cpp	2019-10-31 20:41:56 UTC (rev 251874)
@@ -55,7 +55,6 @@
 #include "Settings.h"
 #include "StringAdaptors.h"
 #include <_javascript_Core/JSCInlines.h>
-#include <_javascript_Core/JSLock.h>
 #include <math.h>
 #include <wtf/IsoMallocInlines.h>
 #include <wtf/RAMSize.h>
@@ -104,20 +103,11 @@
 const unsigned maxCanvasArea = 16384 * 16384;
 #endif
 
-#if USE(CG)
-// FIXME: It seems strange that the default quality is not the one that is literally named "default".
-// Should fix names to make this easier to understand, or write an excellent comment here explaining why not.
-const InterpolationQuality defaultInterpolationQuality = InterpolationLow;
-#else
-const InterpolationQuality defaultInterpolationQuality = InterpolationDefault;
-#endif
-
-static size_t activePixelMemory = 0;
 static size_t maxActivePixelMemoryForTesting = 0;
 
 HTMLCanvasElement::HTMLCanvasElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
-    , m_size(defaultWidth, defaultHeight)
+    , CanvasBase(IntSize(defaultWidth, defaultHeight))
 {
     ASSERT(hasTagName(canvasTag));
 }
@@ -132,24 +122,15 @@
     return adoptRef(*new HTMLCanvasElement(tagName, document));
 }
 
-static void removeFromActivePixelMemory(size_t pixelsReleased)
-{
-    if (!pixelsReleased)
-        return;
-
-    if (pixelsReleased < activePixelMemory)
-        activePixelMemory -= pixelsReleased;
-    else
-        activePixelMemory = 0;
-}
-    
 HTMLCanvasElement::~HTMLCanvasElement()
 {
+    // FIXME: This has to be called here because CSSCanvasValue::CanvasObserverProxy::canvasDestroyed()
+    // downcasts the CanvasBase object to HTMLCanvasElement. That invokes virtual methods, which should be
+    // avoided in destructors, but works as long as it's done before HTMLCanvasElement destructs completely.
     notifyObserversCanvasDestroyed();
 
     m_context = nullptr; // Ensure this goes away before the ImageBuffer.
-
-    releaseImageBufferAndContext();
+    setImageBuffer(nullptr);
 }
 
 void HTMLCanvasElement::parseAttribute(const QualifiedName& name, const AtomString& value)
@@ -193,6 +174,18 @@
     return { };
 }
 
+void HTMLCanvasElement::setSize(const IntSize& newSize)
+{
+    if (newSize == size())
+        return;
+
+    m_ignoreReset = true;
+    setWidth(newSize.width());
+    setHeight(newSize.height());
+    m_ignoreReset = false;
+    reset();
+}
+
 static inline size_t maxActivePixelMemory()
 {
     if (maxActivePixelMemoryForTesting)
@@ -341,7 +334,7 @@
 
     // Make sure we don't use more pixel memory than the system can support.
     size_t requestedPixelMemory = 4 * width() * height();
-    if (activePixelMemory + requestedPixelMemory > maxActivePixelMemory()) {
+    if (activePixelMemory() + requestedPixelMemory > maxActivePixelMemory()) {
         StringBuilder stringBuilder;
         stringBuilder.appendLiteral("Total canvas memory use exceeds the maximum limit (");
         stringBuilder.appendNumber(maxActivePixelMemory() / 1024 / 1024);
@@ -547,12 +540,7 @@
     int w = limitToOnlyHTMLNonNegative(attributeWithoutSynchronization(widthAttr), defaultWidth);
     int h = limitToOnlyHTMLNonNegative(attributeWithoutSynchronization(heightAttr), defaultHeight);
 
-    if (m_contextStateSaver) {
-        // Reset to the initial graphics context state.
-        m_contextStateSaver->restore();
-        m_contextStateSaver->save();
-    }
-
+    resetGraphicsContextState();
     if (is<CanvasRenderingContext2D>(m_context.get()))
         downcast<CanvasRenderingContext2D>(*m_context).reset();
 
@@ -640,12 +628,6 @@
     return m_context && m_context->isGPUBased();
 }
 
-void HTMLCanvasElement::makeRenderingResultsAvailable()
-{
-    if (m_context)
-        m_context->paintRenderingResultsToCanvas();
-}
-
 void HTMLCanvasElement::makePresentationCopy()
 {
     if (!m_presentedImage) {
@@ -659,17 +641,11 @@
     m_presentedImage = nullptr;
 }
 
-void HTMLCanvasElement::releaseImageBufferAndContext()
-{
-    m_contextStateSaver = nullptr;
-    setImageBuffer(nullptr);
-}
-    
 void HTMLCanvasElement::setSurfaceSize(const IntSize& size)
 {
-    m_size = size;
+    CanvasBase::setSize(size);
     m_hasCreatedImageBuffer = false;
-    releaseImageBufferAndContext();
+    setImageBuffer(nullptr);
     clearCopiedImage();
 }
 
@@ -698,7 +674,7 @@
     if (!originClean())
         return Exception { SecurityError };
 
-    if (m_size.isEmpty() || !buffer())
+    if (size().isEmpty() || !buffer())
         return UncachedString { "data:,"_s };
     if (RuntimeEnabledFeatures::sharedFeatures().webAPIStatisticsEnabled())
         ResourceLoadObserver::shared().logCanvasRead(document());
@@ -727,7 +703,7 @@
     if (!originClean())
         return Exception { SecurityError };
 
-    if (m_size.isEmpty() || !buffer()) {
+    if (size().isEmpty() || !buffer()) {
         callback->scheduleCallback(context, nullptr);
         return { };
     }
@@ -840,28 +816,6 @@
 #endif
 }
 
-size_t HTMLCanvasElement::memoryCost() const
-{
-    // memoryCost() may be invoked concurrently from a GC thread, and we need to be careful
-    // about what data we access here and how. We need to hold a lock to prevent m_imageBuffer
-    // from being changed while we access it.
-    auto locker = holdLock(m_imageBufferAssignmentLock);
-    if (!m_imageBuffer)
-        return 0;
-    return m_imageBuffer->memoryCost();
-}
-
-size_t HTMLCanvasElement::externalMemoryCost() const
-{
-    // externalMemoryCost() may be invoked concurrently from a GC thread, and we need to be careful
-    // about what data we access here and how. We need to hold a lock to prevent m_imageBuffer
-    // from being changed while we access it.
-    auto locker = holdLock(m_imageBufferAssignmentLock);
-    if (!m_imageBuffer)
-        return 0;
-    return m_imageBuffer->externalMemoryCost();
-}
-
 void HTMLCanvasElement::setUsesDisplayListDrawing(bool usesDisplayListDrawing)
 {
     if (usesDisplayListDrawing == m_usesDisplayListDrawing)
@@ -902,7 +856,7 @@
 
 void HTMLCanvasElement::createImageBuffer() const
 {
-    ASSERT(!m_imageBuffer);
+    ASSERT(!hasCreatedImageBuffer());
 
     m_hasCreatedImageBuffer = true;
     m_didClearImageBuffer = true;
@@ -919,7 +873,7 @@
     
     // Make sure we don't use more pixel memory than the system can support.
     size_t requestedPixelMemory = 4 * width() * height();
-    if (activePixelMemory + requestedPixelMemory > maxActivePixelMemory()) {
+    if (activePixelMemory() + requestedPixelMemory > maxActivePixelMemory()) {
         StringBuilder stringBuilder;
         stringBuilder.appendLiteral("Total canvas memory use exceeds the maximum limit (");
         stringBuilder.appendNumber(maxActivePixelMemory() / 1024 / 1024);
@@ -935,38 +889,7 @@
 
     auto hostWindow = (document().view() && document().view()->root()) ? document().view()->root()->hostWindow() : nullptr;
     setImageBuffer(ImageBuffer::create(size(), renderingMode, 1, ColorSpaceSRGB, hostWindow));
-}
 
-void HTMLCanvasElement::setImageBuffer(std::unique_ptr<ImageBuffer>&& buffer) const
-{
-    size_t previousMemoryCost = memoryCost();
-    removeFromActivePixelMemory(previousMemoryCost);
-
-    {
-        auto locker = holdLock(m_imageBufferAssignmentLock);
-        m_contextStateSaver = nullptr;
-        m_imageBuffer = WTFMove(buffer);
-    }
-
-    if (m_imageBuffer && m_size != m_imageBuffer->internalSize())
-        m_size = m_imageBuffer->internalSize();
-
-    size_t currentMemoryCost = memoryCost();
-    activePixelMemory += currentMemoryCost;
-
-    if (m_context && m_imageBuffer && previousMemoryCost != currentMemoryCost)
-        InspectorInstrumentation::didChangeCanvasMemory(*m_context);
-
-    if (!m_imageBuffer)
-        return;
-    m_imageBuffer->context().setShadowsIgnoreTransforms(true);
-    m_imageBuffer->context().setImageInterpolationQuality(defaultInterpolationQuality);
-    m_imageBuffer->context().setStrokeThickness(1);
-    m_contextStateSaver = makeUnique<GraphicsContextStateSaver>(m_imageBuffer->context());
-
-    JSC::JSLockHolder lock(HTMLElement::scriptExecutionContext()->vm());
-    HTMLElement::scriptExecutionContext()->vm().heap.reportExtraMemoryAllocated(memoryCost());
-
 #if USE(IOSURFACE_CANVAS_BACKING_STORE) || ENABLE(ACCELERATED_2D_CANVAS)
     if (m_context && m_context->is2d()) {
         // Recalculate compositing requirements if acceleration state changed.
@@ -979,32 +902,9 @@
 {
     m_hasCreatedImageBuffer = true;
     setImageBuffer(WTFMove(buffer));
-    didDraw(FloatRect(FloatPoint(), m_size));
+    didDraw(FloatRect(FloatPoint(), size()));
 }
 
-GraphicsContext* HTMLCanvasElement::drawingContext() const
-{
-    if (m_context && !m_context->is2d())
-        return nullptr;
-
-    return buffer() ? &m_imageBuffer->context() : nullptr;
-}
-
-GraphicsContext* HTMLCanvasElement::existingDrawingContext() const
-{
-    if (!m_hasCreatedImageBuffer)
-        return nullptr;
-
-    return drawingContext();
-}
-
-ImageBuffer* HTMLCanvasElement::buffer() const
-{
-    if (!m_hasCreatedImageBuffer)
-        createImageBuffer();
-    return m_imageBuffer.get();
-}
-
 Image* HTMLCanvasElement::copiedImage() const
 {
     if (!m_copiedImage && buffer()) {
@@ -1035,10 +935,4 @@
     m_didClearImageBuffer = false;
 }
 
-AffineTransform HTMLCanvasElement::baseTransform() const
-{
-    ASSERT(m_hasCreatedImageBuffer);
-    return m_imageBuffer->baseTransform();
 }
-
-}

Modified: trunk/Source/WebCore/html/HTMLCanvasElement.h (251873 => 251874)


--- trunk/Source/WebCore/html/HTMLCanvasElement.h	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/HTMLCanvasElement.h	2019-10-31 20:41:56 UTC (rev 251874)
@@ -31,7 +31,6 @@
 #include "FloatRect.h"
 #include "HTMLElement.h"
 #include "ImageBitmapRenderingContextSettings.h"
-#include "IntSize.h"
 #include <memory>
 #include <wtf/Forward.h>
 
@@ -42,9 +41,9 @@
 namespace WebCore {
 
 class BlobCallback;
+class CanvasRenderingContext;
 class CanvasRenderingContext2D;
 class GraphicsContext;
-class GraphicsContextStateSaver;
 class Image;
 class ImageBuffer;
 class ImageData;
@@ -65,25 +64,12 @@
     static Ref<HTMLCanvasElement> create(const QualifiedName&, Document&);
     virtual ~HTMLCanvasElement();
 
-    unsigned width() const final { return size().width(); }
-    unsigned height() const final { return size().height(); }
-
     WEBCORE_EXPORT ExceptionOr<void> setWidth(unsigned);
     WEBCORE_EXPORT ExceptionOr<void> setHeight(unsigned);
 
-    const IntSize& size() const final { return m_size; }
+    void setSize(const IntSize& newSize) override;
 
-    void setSize(const IntSize& newSize) override
-    { 
-        if (newSize == size())
-            return;
-        m_ignoreReset = true; 
-        setWidth(newSize.width());
-        setHeight(newSize.height());
-        m_ignoreReset = false;
-        reset();
-    }
-
+    CanvasRenderingContext* renderingContext() const final { return m_context.get(); }
     ExceptionOr<Optional<RenderingContext>> getContext(JSC::JSGlobalObject&, const String& contextId, Vector<JSC::Strong<JSC::Unknown>>&& arguments);
 
     CanvasRenderingContext* getContext(const String&);
@@ -116,15 +102,11 @@
 
     void paint(GraphicsContext&, const LayoutRect&);
 
-    GraphicsContext* drawingContext() const final;
-    GraphicsContext* existingDrawingContext() const final;
-
 #if ENABLE(MEDIA_STREAM)
     RefPtr<MediaSample> toMediaSample();
     ExceptionOr<Ref<MediaStream>> captureStream(Document&, Optional<double>&& frameRequestRate);
 #endif
 
-    ImageBuffer* buffer() const;
     Image* copiedImage() const final;
     void clearCopiedImage();
     RefPtr<ImageData> getImageData();
@@ -133,11 +115,6 @@
 
     SecurityOrigin* securityOrigin() const final;
 
-    AffineTransform baseTransform() const final;
-
-    void makeRenderingResultsAvailable() final;
-    bool hasCreatedImageBuffer() const { return m_hasCreatedImageBuffer; }
-
     bool shouldAccelerate(const IntSize&) const;
 
     WEBCORE_EXPORT void setUsesDisplayListDrawing(bool);
@@ -145,9 +122,6 @@
     WEBCORE_EXPORT String displayListAsText(DisplayList::AsTextFlags) const;
     WEBCORE_EXPORT String replayDisplayListAsText(DisplayList::AsTextFlags) const;
 
-    size_t memoryCost() const;
-    size_t externalMemoryCost() const;
-
     // FIXME: Only some canvas rendering contexts need an ImageBuffer.
     // It would be better to have the contexts own the buffers.
     void setImageBufferAndMarkDirty(std::unique_ptr<ImageBuffer>&&);
@@ -167,12 +141,12 @@
 
     void reset();
 
-    void createImageBuffer() const;
+    void createImageBuffer() const final;
     void clearImageBuffer() const;
 
+    bool hasCreatedImageBuffer() const final { return m_hasCreatedImageBuffer; }
+
     void setSurfaceSize(const IntSize&);
-    void setImageBuffer(std::unique_ptr<ImageBuffer>&&) const;
-    void releaseImageBufferAndContext();
 
     bool paintsIntoCanvasBuffer() const;
 
@@ -184,7 +158,6 @@
     ScriptExecutionContext* canvasBaseScriptExecutionContext() const final { return HTMLElement::scriptExecutionContext(); }
 
     FloatRect m_dirtyRect;
-    mutable IntSize m_size;
 
     bool m_ignoreReset { false };
 
@@ -191,14 +164,12 @@
     bool m_usesDisplayListDrawing { false };
     bool m_tracksDisplayListReplay { false };
 
-    mutable Lock m_imageBufferAssignmentLock;
-    
-    // m_createdImageBuffer means we tried to malloc the buffer.  We didn't necessarily get it.
+    std::unique_ptr<CanvasRenderingContext> m_context;
+
+    // m_hasCreatedImageBuffer means we tried to malloc the buffer. We didn't necessarily get it.
     mutable bool m_hasCreatedImageBuffer { false };
     mutable bool m_didClearImageBuffer { false };
-    mutable std::unique_ptr<ImageBuffer> m_imageBuffer;
-    mutable std::unique_ptr<GraphicsContextStateSaver> m_contextStateSaver;
-    
+
     mutable RefPtr<Image> m_presentedImage;
     mutable RefPtr<Image> m_copiedImage; // FIXME: This is temporary for platforms that have to copy the image buffer to render (and for CSSCanvasValue).
 };

Modified: trunk/Source/WebCore/html/OffscreenCanvas.cpp (251873 => 251874)


--- trunk/Source/WebCore/html/OffscreenCanvas.cpp	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/OffscreenCanvas.cpp	2019-10-31 20:41:56 UTC (rev 251874)
@@ -44,8 +44,8 @@
 }
 
 OffscreenCanvas::OffscreenCanvas(ScriptExecutionContext& context, unsigned width, unsigned height)
-    : ContextDestructionObserver(&context)
-    , m_size(width, height)
+    : CanvasBase(IntSize(width, height))
+    , ContextDestructionObserver(&context)
 {
 }
 
@@ -53,39 +53,20 @@
 {
     notifyObserversCanvasDestroyed();
 
-    m_context = nullptr;
+    m_context = nullptr; // Ensure this goes away before the ImageBuffer.
+    setImageBuffer(nullptr);
 }
 
-unsigned OffscreenCanvas::width() const
-{
-    return m_size.width();
-}
-
 void OffscreenCanvas::setWidth(unsigned newWidth)
 {
-    return m_size.setWidth(newWidth);
+    setSize(IntSize(newWidth, height()));
 }
 
-unsigned OffscreenCanvas::height() const
-{
-    return m_size.height();
-}
-
 void OffscreenCanvas::setHeight(unsigned newHeight)
 {
-    return m_size.setHeight(newHeight);
+    setSize(IntSize(width(), newHeight));
 }
 
-const IntSize& OffscreenCanvas::size() const
-{
-    return m_size;
-}
-
-void OffscreenCanvas::setSize(const IntSize& newSize)
-{
-    m_size = newSize;
-}
-
 #if ENABLE(WEBGL)
 ExceptionOr<OffscreenRenderingContext> OffscreenCanvas::getContext(JSC::JSGlobalObject& state, RenderingContextType contextType, Vector<JSC::Strong<JSC::Unknown>>&& arguments)
 {
@@ -123,7 +104,7 @@
     // store from this canvas (or its context), but for now we'll just
     // create a new bitmap and paint into it.
 
-    auto imageBitmap = ImageBitmap::create(m_size);
+    auto imageBitmap = ImageBitmap::create(size());
     if (!imageBitmap->buffer())
         return nullptr;
 
@@ -146,6 +127,10 @@
 #endif
 }
 
+void OffscreenCanvas::createImageBuffer() const
+{
 }
 
+}
+
 #endif

Modified: trunk/Source/WebCore/html/OffscreenCanvas.h (251873 => 251874)


--- trunk/Source/WebCore/html/OffscreenCanvas.h	2019-10-31 20:36:49 UTC (rev 251873)
+++ trunk/Source/WebCore/html/OffscreenCanvas.h	2019-10-31 20:41:56 UTC (rev 251874)
@@ -41,6 +41,7 @@
 
 namespace WebCore {
 
+class CanvasRenderingContext;
 class ImageBitmap;
 class WebGLRenderingContext;
 
@@ -65,13 +66,10 @@
     static Ref<OffscreenCanvas> create(ScriptExecutionContext&, unsigned width, unsigned height);
     virtual ~OffscreenCanvas();
 
-    unsigned width() const final;
     void setWidth(unsigned);
-    unsigned height() const final;
     void setHeight(unsigned);
 
-    const IntSize& size() const final;
-    void setSize(const IntSize&) final;
+    CanvasRenderingContext* renderingContext() const final { return m_context.get(); }
 
 #if ENABLE(WEBGL)
     ExceptionOr<OffscreenRenderingContext> getContext(JSC::JSGlobalObject&, RenderingContextType, Vector<JSC::Strong<JSC::Unknown>>&& arguments);
@@ -79,13 +77,8 @@
     RefPtr<ImageBitmap> transferToImageBitmap();
     // void convertToBlob(ImageEncodeOptions options);
 
-    GraphicsContext* drawingContext() const final { return nullptr; }
-    GraphicsContext* existingDrawingContext() const final { return nullptr; }
-
-    void makeRenderingResultsAvailable() final { }
     void didDraw(const FloatRect&) final { }
 
-    AffineTransform baseTransform() const final { return { }; }
     Image* copiedImage() const final { return nullptr; }
 
     using RefCounted::ref;
@@ -107,7 +100,9 @@
     void refCanvasBase() final { ref(); }
     void derefCanvasBase() final { deref(); }
 
-    IntSize m_size;
+    void createImageBuffer() const final;
+
+    std::unique_ptr<CanvasRenderingContext> m_context;
 };
 
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to