<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 c1bfede..90583ec 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 e77dfe5..378b73a 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 9c9a844..16a7c1f 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 93ee274..f50ea42 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 779122a..479d57a 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 f8cc894..cb8edc2 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 0ca15b9..1f09a76 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 206744e..48c4aae 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 156bf10..e18161b 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