<rdar://problem/9215280> Detached canvas draws with incorrect font
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Apr 2011 06:29:26 +0000 (06:29 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Apr 2011 06:29:26 +0000 (06:29 +0000)
Reviewed by Beth Dakin.

Source/WebCore:

Test: fast/canvas/font-update.html

The existing mechanism for updating the font in a canvas 2D context was lacking in at least
two ways: it neglected to update fonts in all but the topmost state in the stack, and since it
was based on HTMLCanvasElemen's attach() and recalcStyle(), it did not work when the element
was not attached.

This change takes the responsibility for font updates away from the canvas element and gives it
to the canvas context and its graphics state.

* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::registerForInvalidationCallbacks): Added. Adds to the set of registered
font selector clients.
(WebCore::CSSFontSelector::unregisterForInvalidationCallbacks): Added. Removes from the set of
registered font selector clients.
(WebCore::CSSFontSelector::dispatchInvalidationCallbacks): Calls fontsNeedUpdate() on all registered
clients and forces a style recalc on the document.
(WebCore::CSSFontSelector::fontLoaded): Changed to call dispatchInvalidationCallbacks().
(WebCore::CSSFontSelector::fontCacheInvalidated): Ditto.
* css/CSSFontSelector.h:
* html/HTMLCanvasElement.cpp: Removed overrides of attach() and recalcStyle().
* html/HTMLCanvasElement.h:
* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::State::~State): Added. Unregisters with the font selector.
(WebCore::CanvasRenderingContext2D::State::fontsNeedUpdate): Added. Called by the font selector
when its fonts need to be updated. Updates the font.
(WebCore::CanvasRenderingContext2D::setFont): Registers the state with the font selector.
* html/canvas/CanvasRenderingContext2D.h:
* platform/graphics/FontSelector.h:
(WebCore::FontSelectorClient::~FontSelectorClient):

LayoutTests:

* fast/canvas/font-update-expected.checksum: Added.
* fast/canvas/font-update-expected.png: Added.
* fast/canvas/font-update-expected.txt: Added.
* fast/canvas/font-update.html: Added.

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/canvas/font-update-expected.checksum [new file with mode: 0644]
LayoutTests/fast/canvas/font-update-expected.png [new file with mode: 0644]
LayoutTests/fast/canvas/font-update-expected.txt [new file with mode: 0644]
LayoutTests/fast/canvas/font-update.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontSelector.cpp
Source/WebCore/css/CSSFontSelector.h
Source/WebCore/html/HTMLCanvasElement.cpp
Source/WebCore/html/HTMLCanvasElement.h
Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
Source/WebCore/html/canvas/CanvasRenderingContext2D.h
Source/WebCore/platform/graphics/FontSelector.h

index c1bfede8174d911c09f14515f940dcedf42f9043..90583ec746971ac3495f1c8838fc9fb9c74cce00 100644 (file)
@@ -1,3 +1,14 @@
+2011-04-09  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        <rdar://problem/9215280> Detached canvas draws with incorrect font
+
+        * fast/canvas/font-update-expected.checksum: Added.
+        * fast/canvas/font-update-expected.png: Added.
+        * fast/canvas/font-update-expected.txt: Added.
+        * fast/canvas/font-update.html: Added.
+
 2011-04-09  Dirk Pranke  <dpranke@chromium.org>
 
         Unreviewed, expectations change.
diff --git a/LayoutTests/fast/canvas/font-update-expected.checksum b/LayoutTests/fast/canvas/font-update-expected.checksum
new file mode 100644 (file)
index 0000000..81e014f
--- /dev/null
@@ -0,0 +1 @@
+5645ecb5602f926d323bb26efd36c16f
\ No newline at end of file
diff --git a/LayoutTests/fast/canvas/font-update-expected.png b/LayoutTests/fast/canvas/font-update-expected.png
new file mode 100644 (file)
index 0000000..1b0cd81
Binary files /dev/null and b/LayoutTests/fast/canvas/font-update-expected.png differ
diff --git a/LayoutTests/fast/canvas/font-update-expected.txt b/LayoutTests/fast/canvas/font-update-expected.txt
new file mode 100644 (file)
index 0000000..f5edaf5
--- /dev/null
@@ -0,0 +1,8 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderText {#text} at (0,0) size 0x0
+      RenderText {#text} at (0,0) size 0x0
+      RenderHTMLCanvas {CANVAS} at (0,0) size 300x150
diff --git a/LayoutTests/fast/canvas/font-update.html b/LayoutTests/fast/canvas/font-update.html
new file mode 100644 (file)
index 0000000..36fa6a2
--- /dev/null
@@ -0,0 +1,25 @@
+<style>
+    @font-face {
+        font-family: no-such-font;
+        src: url(no-such-file.ttf);
+    }
+</style>
+<canvas id="target"></canvas>
+<script>
+    canvas = document.getElementById("target");
+    ctx = canvas.getContext('2d');
+    ctx.font = "100px no-such-font, ahem";
+    ctx.fillStyle = "red";
+    ctx.fillText("B", 0, 100);
+    ctx.fillStyle = "green";
+    canvas.parentNode.removeChild(canvas);
+    if (window.layoutTestController)
+        layoutTestController.waitUntilDone();
+    setTimeout(function()
+    {
+        ctx.fillText("A", 0, 100);
+        document.body.appendChild(canvas);
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }, 50);
+</script>
index e77dfe5f345e67150c03cd8049c8364c9867ab2d..378b73a02bfd429d90dffe11b48a603ca7966997 100644 (file)
@@ -1,3 +1,40 @@
+2011-04-09  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Beth Dakin.
+
+        <rdar://problem/9215280> Detached canvas draws with incorrect font
+
+        Test: fast/canvas/font-update.html
+
+        The existing mechanism for updating the font in a canvas 2D context was lacking in at least
+        two ways: it neglected to update fonts in all but the topmost state in the stack, and since it
+        was based on HTMLCanvasElemen's attach() and recalcStyle(), it did not work when the element
+        was not attached.
+
+        This change takes the responsibility for font updates away from the canvas element and gives it
+        to the canvas context and its graphics state.
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::registerForInvalidationCallbacks): Added. Adds to the set of registered
+        font selector clients.
+        (WebCore::CSSFontSelector::unregisterForInvalidationCallbacks): Added. Removes from the set of
+        registered font selector clients.
+        (WebCore::CSSFontSelector::dispatchInvalidationCallbacks): Calls fontsNeedUpdate() on all registered
+        clients and forces a style recalc on the document.
+        (WebCore::CSSFontSelector::fontLoaded): Changed to call dispatchInvalidationCallbacks().
+        (WebCore::CSSFontSelector::fontCacheInvalidated): Ditto.
+        * css/CSSFontSelector.h:
+        * html/HTMLCanvasElement.cpp: Removed overrides of attach() and recalcStyle().
+        * html/HTMLCanvasElement.h:
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::State::~State): Added. Unregisters with the font selector.
+        (WebCore::CanvasRenderingContext2D::State::fontsNeedUpdate): Added. Called by the font selector
+        when its fonts need to be updated. Updates the font.
+        (WebCore::CanvasRenderingContext2D::setFont): Registers the state with the font selector.
+        * html/canvas/CanvasRenderingContext2D.h:
+        * platform/graphics/FontSelector.h:
+        (WebCore::FontSelectorClient::~FontSelectorClient):
+
 2011-04-09  Geoffrey Garen  <ggaren@apple.com>
 
         Not reviewed.
index 9c9a844863c46075300a12b81f767ce70d2b0771..16a7c1fa299b6f5e5a5b51d196c2e917f2cd26e5 100644 (file)
@@ -345,18 +345,37 @@ void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule)
     }
 }
 
-void CSSFontSelector::fontLoaded()
+void CSSFontSelector::registerForInvalidationCallbacks(FontSelectorClient* client)
+{
+    m_clients.add(client);
+}
+
+void CSSFontSelector::unregisterForInvalidationCallbacks(FontSelectorClient* client)
 {
+    m_clients.remove(client);
+}
+
+void CSSFontSelector::dispatchInvalidationCallbacks()
+{
+    Vector<FontSelectorClient*> clients;
+    copyToVector(m_clients, clients);
+    for (size_t i = 0; i < clients.size(); ++i)
+        clients[i]->fontsNeedUpdate(this);
+
+    // FIXME: Make Document a FontSelectorClient so that it can simply register for invalidation callbacks.
     if (!m_document || m_document->inPageCache() || !m_document->renderer())
         return;
     m_document->scheduleForcedStyleRecalc();
 }
 
+void CSSFontSelector::fontLoaded()
+{
+    dispatchInvalidationCallbacks();
+}
+
 void CSSFontSelector::fontCacheInvalidated()
 {
-    if (!m_document || m_document->inPageCache() || !m_document->renderer())
-        return;
-    m_document->scheduleForcedStyleRecalc();
+    dispatchInvalidationCallbacks();
 }
 
 static FontData* fontDataForGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
index 93ee274a7748bd38fd85fb106108758c4056d4a2..f50ea42c299301eec2b941a7f7e704f6a03e53f9 100644 (file)
@@ -29,6 +29,7 @@
 #include "FontSelector.h"
 #include <wtf/Forward.h>
 #include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
 #include <wtf/RefPtr.h>
 #include <wtf/text/StringHash.h>
 
@@ -62,13 +63,19 @@ public:
 
     CachedResourceLoader* cachedResourceLoader() const;
 
+    virtual void registerForInvalidationCallbacks(FontSelectorClient*);
+    virtual void unregisterForInvalidationCallbacks(FontSelectorClient*);
+
 private:
     CSSFontSelector(Document*);
 
+    void dispatchInvalidationCallbacks();
+
     Document* m_document;
     HashMap<String, Vector<RefPtr<CSSFontFace> >*, CaseFoldingHash> m_fontFaces;
     HashMap<String, Vector<RefPtr<CSSFontFace> >*, CaseFoldingHash> m_locallyInstalledFontFaces;
     HashMap<String, HashMap<unsigned, RefPtr<CSSSegmentedFontFace> >*, CaseFoldingHash> m_fonts;
+    HashSet<FontSelectorClient*> m_clients;
 };
 
 } // namespace WebCore
index 779122a8021bf9fefb4d0ec0421e8d697d6850b1..479d57a67f02dfcfda063d6c120818e7103eef9c 100644 (file)
@@ -318,27 +318,6 @@ void HTMLCanvasElement::clearPresentationCopy()
     m_presentedImage.clear();
 }
 
-void HTMLCanvasElement::attach()
-{
-    HTMLElement::attach();
-
-    if (m_context && m_context->is2d()) {
-        CanvasRenderingContext2D* ctx = static_cast<CanvasRenderingContext2D*>(m_context.get());
-        ctx->updateFont();
-    }
-}
-
-void HTMLCanvasElement::recalcStyle(StyleChange change)
-{
-    HTMLElement::recalcStyle(change);
-
-    // Update font if needed.
-    if (change == Force && m_context && m_context->is2d()) {
-        CanvasRenderingContext2D* ctx = static_cast<CanvasRenderingContext2D*>(m_context.get());
-        ctx->updateFont();
-    }
-}
-
 void HTMLCanvasElement::setSurfaceSize(const IntSize& size)
 {
     m_size = size;
index f8cc894978a8f14977b4a9a00b6d0e2639daa821..cb8edc20ea4bf1b59bb68c7e98ea80e0cbcff243 100644 (file)
@@ -134,10 +134,6 @@ private:
     virtual void parseMappedAttribute(Attribute*);
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
 
-    virtual void attach();
-
-    virtual void recalcStyle(StyleChange);
-
     void reset();
 
     void createImageBuffer() const;
index 0ca15b9b8c6aa7dac5e3a09b0ece574af63024a2..1f09a762efd4fa57e91e13a8a0193222863d033c 100644 (file)
@@ -213,6 +213,20 @@ CanvasRenderingContext2D::State::State()
 {
 }
 
+CanvasRenderingContext2D::State::~State()
+{
+    if (m_realizedFont)
+        m_font.fontSelector()->unregisterForInvalidationCallbacks(this);
+}
+
+void CanvasRenderingContext2D::State::fontsNeedUpdate(FontSelector* fontSelector)
+{
+    ASSERT_ARG(fontSelector, fontSelector == m_font.fontSelector());
+    ASSERT(m_realizedFont);
+
+    m_font.update(fontSelector);
+}
+
 void CanvasRenderingContext2D::save()
 {
     ASSERT(m_stateStack.size() >= 1);
@@ -1690,15 +1704,7 @@ void CanvasRenderingContext2D::setFont(const String& newFont)
     state().m_font = newStyle->font();
     state().m_font.update(styleSelector->fontSelector());
     state().m_realizedFont = true;
-}
-
-void CanvasRenderingContext2D::updateFont()
-{
-    if (!state().m_realizedFont)
-        return;
-
-    const Font& font = state().m_font;
-    font.update(font.fontSelector());
+    styleSelector->fontSelector()->registerForInvalidationCallbacks(&state());
 }
 
 String CanvasRenderingContext2D::textAlign() const
index 206744efb1b7d53129246c73413f9eba0fccb4aa..48c4aae381442efe8ac6bbb2e1d143323e171df5 100644 (file)
@@ -204,7 +204,6 @@ public:
 
     String font() const;
     void setFont(const String&);
-    void updateFont();
 
     String textAlign() const;
     void setTextAlign(const String&);
@@ -228,8 +227,11 @@ public:
 #endif
 
 private:
-    struct State {
+    struct State : FontSelectorClient {
         State();
+        virtual ~State();
+
+        virtual void fontsNeedUpdate(FontSelector*);
 
         String m_unparsedStrokeColor;
         String m_unparsedFillColor;
index 156bf10fd467e9a7286029c42ea46b67ea3dd25e..e18161bb8f5dddff6bf6ca4abd81e73a79dcb897 100644 (file)
@@ -33,6 +33,7 @@ namespace WebCore {
 
 class FontData;
 class FontDescription;
+class FontSelectorClient;
 
 class FontSelector : public RefCounted<FontSelector> {
 public:
@@ -40,6 +41,16 @@ public:
     virtual FontData* getFontData(const FontDescription&, const AtomicString& familyName) = 0;
 
     virtual void fontCacheInvalidated() { }
+
+    virtual void registerForInvalidationCallbacks(FontSelectorClient*) = 0;
+    virtual void unregisterForInvalidationCallbacks(FontSelectorClient*) = 0;
+};
+
+class FontSelectorClient {
+public:
+    virtual ~FontSelectorClient() { }
+
+    virtual void fontsNeedUpdate(FontSelector*) = 0;
 };
 
 } // namespace WebCore