Improve shadow support in EllipsisBox.

Source/core/rendering/EllipsisBox.cpp

 /**
  * Copyright (C) 2003, 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 12 matching lines @@
 #include "core/dom/Document.h"
 #include "core/platform/graphics/Font.h"
 #include "core/platform/graphics/GraphicsContextStateSaver.h"
 #include "core/platform/graphics/TextRun.h"
 #include "core/rendering/HitTestResult.h"
 #include "core/rendering/InlineTextBox.h"
 #include "core/rendering/PaintInfo.h"
 #include "core/rendering/RenderBlock.h"
 #include "core/rendering/RootInlineBox.h"
 
+#include <algorithm>
+
 namespace WebCore {
 
 void EllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom)
 {
     GraphicsContext* context = paintInfo.context;
     RenderStyle* style = m_renderer->style(isFirstLineStyle());
     Color textColor = style->visitedDependentColor(CSSPropertyWebkitTextFillColor);

[Stephen Chennney, 2013/05/16 17:03:14]

Rename this "defaultTextColor".

[jbroman, 2013/05/16 17:22:47]

Changed to "styleTextColor" to be clear that this arises from the style, not
some global default.

     if (textColor != context->fillColor())
         context->setFillColor(textColor, style->colorSpace());
-    bool setShadow = false;
-    if (style->textShadow()) {
-        context->setShadow(LayoutSize(style->textShadow()->x(), style->textShadow()->y()),
-                           style->textShadow()->blur(), style->textShadow()->color(), style->colorSpace());
-        setShadow = true;
-    }
 
+    Color actualTextColor = textColor;

[Stephen Chennney, 2013/05/16 17:03:14]

Then name this "textColor".

[jbroman, 2013/05/16 17:22:47]

Done.

     const Font& font = style->font();
     if (selectionState() != RenderObject::SelectionNone) {
         paintSelection(context, paintOffset, style, font);
 
         // Select the correct color for painting the text.
         Color foreground = paintInfo.forceBlackText() ? Color::black : renderer()->selectionForegroundColor();
         if (foreground.isValid() && foreground != textColor)
-            context->setFillColor(foreground, style->colorSpace());
+            actualTextColor = foreground;
     }
 
     // FIXME: Why is this always LTR? Fix by passing correct text run flags below.
+    const ShadowData* shadow = style->textShadow();
     FloatPoint boxOrigin(paintOffset);
     boxOrigin.move(x(), y());
+    FloatRect boxRect(boxOrigin, LayoutSize(logicalWidth(), logicalHeight()));
     FloatPoint textOrigin(boxOrigin.x(), boxOrigin.y() + style->fontMetrics().ascent());
     TextRun textRun = RenderBlock::constructTextRun(renderer(), font, m_str, style, TextRun::AllowTrailingExpansion);
     TextRunPaintInfo textRunPaintInfo(textRun);
-    textRunPaintInfo.bounds = FloatRect(boxOrigin, FloatSize(logicalWidth(), logicalHeight()));
-    context->drawText(font, textRunPaintInfo, textOrigin);
+    textRunPaintInfo.bounds = boxRect;
+
+    bool opaque = actualTextColor.alpha() == 255;
+    if (!opaque)
+        context->setFillColor(Color::black, style->colorSpace());
+    else
+        context->setFillColor(actualTextColor, style->colorSpace());
+
+    do {
+        IntSize extraOffset;
+        if (shadow) {
+            FloatSize shadowOffset(shadow->x(), shadow->y());
+            if (shadow->next() || !opaque) {

[Stephen Chennney, 2013/05/16 17:03:14]

What the abstract meaning of this? I would prefer a variable "needToDrawAgain"
or something, describing why you need to take a different path. It also avoids
duplicating the test further down, which is a recipe for bugs if someone ever
makes changes. You can even use it as the while condition.

[jbroman, 2013/05/16 17:22:47]

I've tried to mirror paintTextWithShadows from InlineTextBox.cpp as closely as
possible in order to capture the current behaviour there here. I'm not a huge
fan of the mysterious condition either.

The reason I didn't clean this up here (or in InlineTextBox) is because I have
another CL following this one that does a more dramatic refactor of this code
which would eliminate this condition anyhow.

https://codereview.chromium.org/15137009/diff/18001/Source/core/rendering/Ell...

Would you prefer I come up with some abstract name for this condition in the
meantime, or leave it as-is?

[Stephen Chennney, 2013/05/16 17:34:56]

Now that I've seen the other patch, it's fine to leave this as is.

+                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(actualTextColor, 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);
 
     // Restore the regular fill color.
     if (textColor != context->fillColor())
         context->setFillColor(textColor, style->colorSpace());
 
-    if (setShadow)
-        context->clearShadow();
-
     paintMarkupBox(paintInfo, paintOffset, lineTop, lineBottom, style);
 }
 
 InlineBox* EllipsisBox::markupBox() const
 {
     if (!m_shouldPaintMarkupBox || !m_renderer->isRenderBlock())
         return 0;
 
     RenderBlock* block = toRenderBlock(m_renderer);
     RootInlineBox* lastLine = block->lineAtIndex(block->lineCount() - 1);
@@ skipping 70 matching lines @@
     if (visibleToHitTestRequest(request) && boundsRect.intersects(HitTestLocation::rectForPoint(locationInContainer.point(), 0, 0, 0, 0))) {
         renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(adjustedLocation));
         if (!result.addNodeToRectBasedTestResult(renderer()->node(), request, locationInContainer, boundsRect))
             return true;
     }
 
     return false;
 }
 
 } // namespace WebCore