Changeset 157960 in webkit

Timestamp:

Oct 24, 2013, 4:35:26 PM (13 years ago)

Author:

akling@apple.com

Message:

Uncomplicate some of SVGTextRunRenderingContext.
<​https://webkit.org/b/123294>

This class was weird about a few things, so I've taken these steps
to clear things up:

• FINAL and OVERRIDE all the things.
• Constructor now takes a RenderObject&.
• renderer() now returns a RenderObject&.
• drawSVGGlyphs() no longer takes a TextRun.
• glyphDataForCharacter() no longer takes a TextRun.

To expand on the last two, there was also some awkward hoop-jumping
where we'd go through the TextRun passed by argument to find its
rendering context, which was really just |this| all along.

Reviewed by Antti Koivisto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/FontFastPath.cpp (modified) (2 diffs)
• platform/graphics/TextRun.h (modified) (1 diff)
• platform/graphics/WidthIterator.cpp (modified) (1 diff)
• rendering/InlineTextBox.cpp (modified) (1 diff)
• rendering/RenderBlock.cpp (modified) (2 diffs)
• rendering/svg/SVGInlineTextBox.cpp (modified) (1 diff)
• rendering/svg/SVGTextMetrics.cpp (modified) (1 diff)
• rendering/svg/SVGTextRunRenderingContext.cpp (modified) (7 diffs)
• rendering/svg/SVGTextRunRenderingContext.h (modified) (3 diffs)
• svg/SVGFontData.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-10-24  Andreas Kling  <akling@apple.com>
+
+        Uncomplicate some of SVGTextRunRenderingContext.
+        <https://webkit.org/b/123294>
+
+        This class was weird about a few things, so I've taken these steps
+        to clear things up:
+
+        - FINAL and OVERRIDE all the things.
+        - Constructor now takes a RenderObject&.
+        - renderer() now returns a RenderObject&.
+        - drawSVGGlyphs() no longer takes a TextRun.
+        - glyphDataForCharacter() no longer takes a TextRun.
+
+        To expand on the last two, there was also some awkward hoop-jumping
+        where we'd go through the TextRun passed by argument to find its
+        rendering context, which was really just |this| all along.
+
+        Reviewed by Antti Koivisto.
+
 2013-10-24  Roger Fong  <roger_fong@apple.com>
 

trunk/Source/WebCore/platform/graphics/FontFastPath.cpp

@@ -209 +209 @@
 #if ENABLE(SVG_FONTS)
             if (renderingContext && fontData->isSVGFont())
-                renderingContext->drawSVGGlyphs(context, run, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
+                renderingContext->drawSVGGlyphs(context, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
             else
 #endif
@@ -227 +227 @@
 #if ENABLE(SVG_FONTS)
     if (renderingContext && fontData->isSVGFont())
-        renderingContext->drawSVGGlyphs(context, run, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
+        renderingContext->drawSVGGlyphs(context, fontData, glyphBuffer, lastFrom, nextGlyph - lastFrom, startPoint);
     else
 #endif

trunk/Source/WebCore/platform/graphics/TextRun.h

@@ -213 +213 @@
 
 #if ENABLE(SVG_FONTS)
-        virtual GlyphData glyphDataForCharacter(const Font&, const TextRun&, WidthIterator&, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength) = 0;
-        virtual void drawSVGGlyphs(GraphicsContext*, const TextRun&, const SimpleFontData*, const GlyphBuffer&, int from, int to, const FloatPoint&) const = 0;
+        virtual GlyphData glyphDataForCharacter(const Font&, WidthIterator&, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength) = 0;
+        virtual void drawSVGGlyphs(GraphicsContext*, const SimpleFontData*, const GlyphBuffer&, int from, int to, const FloatPoint&) const = 0;
         virtual float floatWidthUsingSVGFont(const Font&, const TextRun&, int& charsConsumed, String& glyphName) const = 0;
         virtual bool applySVGKerning(const SimpleFontData*, WidthIterator&, GlyphBuffer*, int from) const = 0;

trunk/Source/WebCore/platform/graphics/WidthIterator.cpp

@@ -78 +78 @@
 #if ENABLE(SVG_FONTS)
     if (TextRun::RenderingContext* renderingContext = m_run.renderingContext())
-        return renderingContext->glyphDataForCharacter(*m_font, m_run, *this, character, mirror, currentCharacter, advanceLength);
+        return renderingContext->glyphDataForCharacter(*m_font, *this, character, mirror, currentCharacter, advanceLength);
 #else
     UNUSED_PARAM(currentCharacter);

trunk/Source/WebCore/rendering/InlineTextBox.cpp

@@ -1488 +1488 @@
     run.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
     if (textRunNeedsRenderingContext(font))
-        run.setRenderingContext(SVGTextRunRenderingContext::create(&renderer()));
+        run.setRenderingContext(SVGTextRunRenderingContext::create(renderer()));
 
     // Propagate the maximum length of the characters buffer to the TextRun, even when we're only processing a substring.

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -5433 +5433 @@
 
     TextRun run(characters, length, 0, 0, expansion, textDirection, directionalOverride);
-    if (textRunNeedsRenderingContext(font))
-        run.setRenderingContext(SVGTextRunRenderingContext::create(context));
+    if (textRunNeedsRenderingContext(font)) {
+        ASSERT(context); // FIXME: Thread a RenderObject& to this point so we don't have to dereference anything.
+        run.setRenderingContext(SVGTextRunRenderingContext::create(*context));
+    }
 
     return run;
@@ -5451 +5453 @@
     }
     TextRun run(characters, length, 0, 0, expansion, textDirection, directionalOverride);
-    if (textRunNeedsRenderingContext(font))
-        run.setRenderingContext(SVGTextRunRenderingContext::create(context));
+    if (textRunNeedsRenderingContext(font)) {
+        ASSERT(context); // FIXME: Thread a RenderObject& to this point so we don't have to dereference anything.
+        run.setRenderingContext(SVGTextRunRenderingContext::create(*context));
+    }
 
     return run;

trunk/Source/WebCore/rendering/svg/SVGInlineTextBox.cpp

@@ -418 +418 @@
 
     if (textRunNeedsRenderingContext(style->font()))
-        run.setRenderingContext(SVGTextRunRenderingContext::create(&renderer()));
+        run.setRenderingContext(SVGTextRunRenderingContext::create(renderer()));
 
     run.disableRoundingHacks();

trunk/Source/WebCore/rendering/svg/SVGTextMetrics.cpp

@@ -78 +78 @@
 
     if (textRunNeedsRenderingContext(style->font()))
-        run.setRenderingContext(SVGTextRunRenderingContext::create(text));
+        run.setRenderingContext(SVGTextRunRenderingContext::create(*text));
 
     run.disableRoundingHacks();

trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp

@@ -54 +54 @@
 }
 
-static inline RenderObject* firstParentRendererForNonTextNode(RenderObject* renderer)
-{
-    ASSERT(renderer);
-    return renderer->isText() ? renderer->parent() : renderer;
-}
-
-static inline RenderObject* renderObjectFromRun(const TextRun& run)
-{
-    if (TextRun::RenderingContext* renderingContext = run.renderingContext())
-        return static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
-    return 0;
-}
-
-static inline RenderSVGResource* activePaintingResourceFromRun(const TextRun& run)
-{
-    if (TextRun::RenderingContext* renderingContext = run.renderingContext())
-        return static_cast<SVGTextRunRenderingContext*>(renderingContext)->activePaintingResource();
-    return 0;
-}
-
 float SVGTextRunRenderingContext::floatWidthUsingSVGFont(const Font& font, const TextRun& run, int& charsConsumed, String& glyphName) const
 {
@@ -123 +103 @@
 }
 
-void SVGTextRunRenderingContext::drawSVGGlyphs(GraphicsContext* context, const TextRun& run, const SimpleFontData* fontData, const GlyphBuffer& glyphBuffer, int from, int numGlyphs, const FloatPoint& point) const
+void SVGTextRunRenderingContext::drawSVGGlyphs(GraphicsContext* context, const SimpleFontData* fontData, const GlyphBuffer& glyphBuffer, int from, int numGlyphs, const FloatPoint& point) const
 {
     SVGFontElement* fontElement = 0;
@@ -132 +112 @@
         return;
 
-    // We can only paint SVGFonts if a context is available.
-    RenderSVGResource* activePaintingResource = activePaintingResourceFromRun(run);
-    RenderObject* renderObject = renderObjectFromRun(run);
-    RenderObject* parentRenderObject = firstParentRendererForNonTextNode(renderObject);
-    RenderStyle* parentRenderObjectStyle = 0;
-
-    ASSERT(renderObject);
+    auto activePaintingResource = this->activePaintingResource();
     if (!activePaintingResource) {
         // TODO: We're only supporting simple filled HTML text so far.
@@ -145 +119 @@
         activePaintingResource = solidPaintingResource;
     }
- 
-    bool isVerticalText = false;
-    if (parentRenderObject) {
-        parentRenderObjectStyle = parentRenderObject->style();
-        ASSERT(parentRenderObjectStyle);
-        isVerticalText = parentRenderObjectStyle->svgStyle()->isVerticalWritingMode();
-    }
+
+    auto& elementRenderer = renderer().isRenderElement() ? toRenderElement(renderer()) : *renderer().parent();
+    RenderStyle& style = *elementRenderer.style();
+    bool isVerticalText = style.svgStyle()->isVerticalWritingMode();
 
     float scale = scaleEmToUnits(fontData->platformData().size(), fontFaceElement->unitsPerEm());
@@ -195 +166 @@
         glyphPath.transform(glyphPathTransform);
 
-        if (activePaintingResource->applyResource(parentRenderObject, parentRenderObjectStyle, context, resourceMode)) {
+        if (activePaintingResource->applyResource(&elementRenderer, &style, context, resourceMode)) {
             float strokeThickness = context->strokeThickness();
-            if (renderObject && renderObject->isSVGInlineText())
-                context->setStrokeThickness(strokeThickness * toRenderSVGInlineText(renderObject)->scalingFactor());
-            activePaintingResource->postApplyResource(parentRenderObject, context, resourceMode, &glyphPath, 0);
+            if (renderer().isSVGInlineText())
+                context->setStrokeThickness(strokeThickness * toRenderSVGInlineText(renderer()).scalingFactor());
+            activePaintingResource->postApplyResource(&elementRenderer, context, resourceMode, &glyphPath, 0);
             context->setStrokeThickness(strokeThickness);
         }
@@ -210 +181 @@
 }
 
-GlyphData SVGTextRunRenderingContext::glyphDataForCharacter(const Font& font, const TextRun& run, WidthIterator& iterator, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength)
+GlyphData SVGTextRunRenderingContext::glyphDataForCharacter(const Font& font, WidthIterator& iterator, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength)
 {
     const SimpleFontData* primaryFont = font.primaryFont();
@@ -234 +205 @@
     const SimpleFontData* originalFontData = glyphData.fontData;
     if (glyphData.fontData && !glyphData.fontData->isSVGFont()) {
-        if (TextRun::RenderingContext* renderingContext = run.renderingContext()) {
-            RenderObject* renderObject = static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
-            RenderObject* parentRenderObject = renderObject->isText() ? renderObject->parent() : renderObject;
-            ASSERT(parentRenderObject);
-            if (Element* parentRenderObjectElement = toElement(parentRenderObject->node())) {
-                if (parentRenderObjectElement->hasTagName(SVGNames::altGlyphTag))
-                    glyphData.fontData = primaryFont;
-            }
+        auto& elementRenderer = renderer().isRenderElement() ? toRenderElement(renderer()) : *renderer().parent();
+        if (Element* parentRendererElement = elementRenderer.element()) {
+            if (parentRendererElement->hasTagName(SVGNames::altGlyphTag))
+                glyphData.fontData = primaryFont;
         }
     }

trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.h

@@ -30 +30 @@
 class RenderSVGResource;
 
-class SVGTextRunRenderingContext : public TextRun::RenderingContext {
+class SVGTextRunRenderingContext FINAL : public TextRun::RenderingContext {
 public:
-    static PassRefPtr<SVGTextRunRenderingContext> create(RenderObject* renderer)
+    static PassRef<SVGTextRunRenderingContext> create(RenderObject& renderer)
     {
-        return adoptRef(new SVGTextRunRenderingContext(renderer));
+        return adoptRef(*new SVGTextRunRenderingContext(renderer));
     }
 
-    RenderObject* renderer() const { return m_renderer; }
+    RenderObject& renderer() const { return m_renderer; }
 
 #if ENABLE(SVG_FONTS)
@@ -43 +43 @@
     void setActivePaintingResource(RenderSVGResource* object) { m_activePaintingResource = object; }
 
-    virtual GlyphData glyphDataForCharacter(const Font&, const TextRun&, WidthIterator&, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength);
-    virtual void drawSVGGlyphs(GraphicsContext*, const TextRun&, const SimpleFontData*, const GlyphBuffer&, int from, int to, const FloatPoint&) const;
-    virtual float floatWidthUsingSVGFont(const Font&, const TextRun&, int& charsConsumed, String& glyphName) const;
-    virtual bool applySVGKerning(const SimpleFontData*, WidthIterator&, GlyphBuffer*, int from) const;
+    virtual GlyphData glyphDataForCharacter(const Font&, WidthIterator&, UChar32 character, bool mirror, int currentCharacter, unsigned& advanceLength) OVERRIDE;
+    virtual void drawSVGGlyphs(GraphicsContext*, const SimpleFontData*, const GlyphBuffer&, int from, int to, const FloatPoint&) const OVERRIDE;
+    virtual float floatWidthUsingSVGFont(const Font&, const TextRun&, int& charsConsumed, String& glyphName) const OVERRIDE;
+    virtual bool applySVGKerning(const SimpleFontData*, WidthIterator&, GlyphBuffer*, int from) const OVERRIDE;
 #endif
 
 private:
-    SVGTextRunRenderingContext(RenderObject* renderer)
+    SVGTextRunRenderingContext(RenderObject& renderer)
         : m_renderer(renderer)
 #if ENABLE(SVG_FONTS)
@@ -60 +60 @@
     virtual ~SVGTextRunRenderingContext() { }
 
-    RenderObject* m_renderer;
+    RenderObject& m_renderer;
 
 #if ENABLE(SVG_FONTS)

trunk/Source/WebCore/svg/SVGFontData.cpp

@@ -160 +160 @@
     RenderObject* renderObject = 0;
     if (TextRun::RenderingContext* renderingContext = run.renderingContext())
-        renderObject = static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
+        renderObject = &static_cast<SVGTextRunRenderingContext*>(renderingContext)->renderer();
 
     String language;