Refactor shadow rendering logic.

Source/core/rendering/EllipsisBox.cpp

Index: Source/core/rendering/EllipsisBox.cpp
diff --git a/Source/core/rendering/EllipsisBox.cpp b/Source/core/rendering/EllipsisBox.cpp
index e42366390ddebae0f2cd9d3acc3460ab679c3de4..ca282bf4869d2b7a1ef6cc145546f60e3b1284cd 100644
--- a/Source/core/rendering/EllipsisBox.cpp
+++ b/Source/core/rendering/EllipsisBox.cpp
@@ -21,6 +21,7 @@
 #include "core/rendering/EllipsisBox.h"
 
 #include "core/dom/Document.h"
+#include "core/platform/graphics/DrawLooper.h"
 #include "core/platform/graphics/Font.h"
 #include "core/platform/graphics/GraphicsContextStateSaver.h"
 #include "core/platform/graphics/TextRun.h"
@@ -29,8 +30,8 @@
 #include "core/rendering/PaintInfo.h"
 #include "core/rendering/RenderBlock.h"
 #include "core/rendering/RootInlineBox.h"
-
-#include <algorithm>
+#include "core/rendering/style/ShadowData.h"
+#include "wtf/Vector.h"
 
 namespace WebCore {
 
@@ -50,11 +51,32 @@ void EllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, La
         // Select the correct color for painting the text.
         Color foreground = paintInfo.forceBlackText() ? Color::black : renderer()->selectionForegroundColor();
         if (foreground.isValid() && foreground != styleTextColor)
-            textColor = foreground;
+            context->setFillColor(foreground, style->colorSpace());
     }
 
-    // FIXME: Why is this always LTR? Fix by passing correct text run flags below.
     const ShadowData* shadow = style->textShadow();
+    bool hasShadow = !!shadow;

[Stephen Chennney, 2013/05/17 15:35:34]

Why !!? To avoid a perf warning on Windows?

[jbroman, 2013/05/17 15:43:18]

Some people prefer to add !! to say "Hey, this is a boolean! I'm not
accidentally assigning a pointer to a boolean!". I'm happy to get rid of it.

+    if (hasShadow) {
+        // FIXME: it would be better if we could get the shadows top-to-bottom from the style.
+        Vector<const ShadowData*, 4> shadows;
+        do {
+            shadows.append(shadow);
+        } while ((shadow = shadow->next()));

[Stephen Chennney, 2013/05/17 15:35:34]

Why do you create and fill a vector then loop over it then delete it. Why not
create add the shadows directly while walking the shadow list?

[jbroman, 2013/05/17 15:43:18]

Because ShadowData is a linked list of the shadows from bottom to top, and
SkDrawLooper adds layers from top to bottom. This is what the FIXME is referring
to; it would be nice if I didn't have to reverse the list before adding the
shadows (possibly by having the style resolution code link it in the opposite
order, but I don't know how other uses of ShadowData would be affected by doing
that).

[jbroman, 2013/05/17 19:16:55]

Another, possibly easier option, would be to get a method added to
SkLayerDrawLooper which allows prepending layers. That might prove easier and
more generally useful.

+
+        DrawLooper drawLooper;
+        drawLooper.addUnmodifiedContent();
+        for (int i = shadows.size() - 1; i >= 0; i--) {
+            shadow = shadows[i];
+            int shadowX = isHorizontal() ? shadow->x() : shadow->y();
+            int shadowY = isHorizontal() ? shadow->y() : -shadow->x();
+            FloatSize offset(shadowX, shadowY);
+            drawLooper.addShadow(offset, shadow->blur(), shadow->color(),
+                DrawLooper::ShadowRespectsTransforms, DrawLooper::ShadowIgnoresAlpha);
+        }
+        context->setDrawLooper(drawLooper);
+    }
+
+    // FIXME: Why is this always LTR? Fix by passing correct text run flags below.
     FloatPoint boxOrigin(paintOffset);
     boxOrigin.move(x(), y());
     FloatRect boxRect(boxOrigin, LayoutSize(logicalWidth(), logicalHeight()));
@@ -62,47 +84,15 @@ void EllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, La
     TextRun textRun = RenderBlock::constructTextRun(renderer(), font, m_str, style, TextRun::AllowTrailingExpansion);
     TextRunPaintInfo textRunPaintInfo(textRun);
     textRunPaintInfo.bounds = boxRect;
-
-    bool opaque = textColor.alpha() == 255;
-    if (!opaque)
-        context->setFillColor(Color::black, style->colorSpace());
-    else
-        context->setFillColor(textColor, style->colorSpace());
-
-    do {
-        IntSize extraOffset;
-        if (shadow) {
-            FloatSize shadowOffset(shadow->x(), shadow->y());
-            if (shadow->next() || !opaque) {
-                FloatRect shadowRect(boxRect);
-                shadowRect.inflate(shadow->blur());
-                shadowRect.move(shadowOffset);
-                context->save();
-                context->clip(shadowRect);
-                extraOffset = IntSize(0, 2 * boxRect.height() + std::max(0.0f, shadowOffset.height()) + shadow->blur());
-                shadowOffset -= extraOffset;
-            }
-            context->setShadow(shadowOffset, shadow->blur(), shadow->color(), style->colorSpace());
-        } else if (!opaque)
-            context->setFillColor(textColor, style->colorSpace());
-
-        context->drawText(font, textRunPaintInfo, textOrigin + extraOffset);
-
-        if (!shadow)
-            break;
-
-        if (shadow->next() || !opaque)
-            context->restore();
-        else
-            context->clearShadow();
-
-        shadow = shadow->next();
-    } while (shadow || !opaque);
+    context->drawText(font, textRunPaintInfo, textOrigin);
 
     // Restore the regular fill color.
     if (styleTextColor != context->fillColor())
         context->setFillColor(styleTextColor, style->colorSpace());
 
+    if (hasShadow)
+        context->clearDrawLooper();
+
     paintMarkupBox(paintInfo, paintOffset, lineTop, lineBottom, style);
 }