Proper support for multiple text decorations.

Source/core/rendering/style/RenderStyle.cpp

 /*
  * Copyright (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Adobe Systems Incorporated. All rights reserved.
  *
  * 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.
  *
@@ skipping 10 matching lines @@
  */
 
 #include "config.h"
 #include "core/rendering/style/RenderStyle.h"
 
 #include <algorithm>
 #include "RuntimeEnabledFeatures.h"
 #include "core/css/resolver/StyleResolver.h"
 #include "core/rendering/RenderTheme.h"
 #include "core/rendering/TextAutosizer.h"
+#include "core/rendering/style/AppliedTextDecoration.h"
 #include "core/rendering/style/ContentData.h"
 #include "core/rendering/style/CursorList.h"
 #include "core/rendering/style/QuotesData.h"
 #include "core/rendering/style/ShadowList.h"
 #include "core/rendering/style/StyleImage.h"
 #include "core/rendering/style/StyleInheritedData.h"
 #include "platform/LengthFunctions.h"
 #include "platform/fonts/Font.h"
 #include "platform/fonts/FontSelector.h"
 #include "platform/geometry/FloatRoundedRect.h"
@@ skipping 640 matching lines @@
 
         if (rareNonInheritedData->opacity != other.rareNonInheritedData->opacity)
             changedContextSensitiveProperties |= ContextSensitivePropertyOpacity;
 
         if (rareNonInheritedData->m_filter != other.rareNonInheritedData->m_filter)
             changedContextSensitiveProperties |= ContextSensitivePropertyFilter;
     }
 
     if (!diff.needsRepaint()) {
         if (inherited->color != other.inherited->color
-            || inherited_flags._text_decorations != other.inherited_flags._text_decorations
+            || inherited_flags.m_textUnderline != other.inherited_flags.m_textUnderline
             || visual->textDecoration != other.visual->textDecoration) {
             changedContextSensitiveProperties |= ContextSensitivePropertyTextOrColor;
         } else if (rareNonInheritedData.get() != other.rareNonInheritedData.get()) {
             if (rareNonInheritedData->m_textDecorationStyle != other.rareNonInheritedData->m_textDecorationStyle
                 || rareNonInheritedData->m_textDecorationColor != other.rareNonInheritedData->m_textDecorationColor)
                 changedContextSensitiveProperties |= ContextSensitivePropertyTextOrColor;
         } else if (rareInheritedData.get() != other.rareInheritedData.get()) {
             if (rareInheritedData->textFillColor() != other.rareInheritedData->textFillColor()
                 || rareInheritedData->textStrokeColor() != other.rareInheritedData->textStrokeColor()
                 || rareInheritedData->textEmphasisColor() != other.rareInheritedData->textEmphasisColor()
-                || rareInheritedData->textEmphasisFill != other.rareInheritedData->textEmphasisFill)
+                || rareInheritedData->textEmphasisFill != other.rareInheritedData->textEmphasisFill
+                || rareInheritedData->appliedTextDecorations != other.rareInheritedData->appliedTextDecorations)
                 changedContextSensitiveProperties |= ContextSensitivePropertyTextOrColor;
         }
     }
 
     return changedContextSensitiveProperties;
 }
 
 void RenderStyle::setClip(Length top, Length right, Length bottom, Length left)
 {
     StyleVisualData* data = visual.access();
@@ skipping 484 matching lines @@
 }
 
 const Font& RenderStyle::font() const { return inherited->font; }
 const FontMetrics& RenderStyle::fontMetrics() const { return inherited->font.fontMetrics(); }
 const FontDescription& RenderStyle::fontDescription() const { return inherited->font.fontDescription(); }
 float RenderStyle::specifiedFontSize() const { return fontDescription().specifiedSize(); }
 float RenderStyle::computedFontSize() const { return fontDescription().computedSize(); }
 int RenderStyle::fontSize() const { return fontDescription().computedPixelSize(); }
 FontWeight RenderStyle::fontWeight() const { return fontDescription().weight(); }
 
+TextDecoration RenderStyle::textDecorationsInEffect() const
+{
+    int decorations = 0;
+
+    const Vector<AppliedTextDecoration>& applied = appliedTextDecorations();
+
+    for (size_t i = 0; i < applied.size(); ++i)
+        decorations |= applied[i].line();
+
+    return static_cast<TextDecoration>(decorations);
+}
+
+const Vector<AppliedTextDecoration>& RenderStyle::appliedTextDecorations() const
+{
+    DEFINE_STATIC_LOCAL(Vector<AppliedTextDecoration>, empty, ());
+    DEFINE_STATIC_LOCAL(Vector<AppliedTextDecoration>, underline, (1, AppliedTextDecoration(TextDecorationUnderline)));

[Julien - ping for review, 2014/05/05 17:45:08]

I would prefer the vectors to be defined inside their own branches. This matches
our coding style to define local variable the closest / deepest to where they
are needed.

[andersr, 2014/05/06 13:15:29]

Done.

+
+    if (!inherited_flags.m_textUnderline && !rareInheritedData->appliedTextDecorations)

[Julien - ping for review, 2014/05/05 17:45:08]

This line makes me think that when inherited_flags.m_textUnderline is false,
rareInheritedData->appliedTextDecorations shouldn't be NULL.

[andersr, 2014/05/06 13:15:29]

Do you mean that it does not read well, or that it's logically wrong? (It looks
correct to me). What would you prefer?

!(m_textUnderline || appliedTextDecoration) // This?

+        return empty;
+    if (inherited_flags.m_textUnderline && !rareInheritedData->appliedTextDecorations)

[Julien - ping for review, 2014/05/05 17:45:08]

Wouldn't rareInheritedData->appliedTextDecorations be NULL if
inherited_flags.m_textUnderline is true based on the addAppliedTextDecoration
logic below?

[andersr, 2014/05/06 13:15:29]

That's true. Fixed.

+        return underline;
+
+    return rareInheritedData->appliedTextDecorations->vector();
+}
+
 float RenderStyle::wordSpacing() const { return fontDescription().wordSpacing(); }
 float RenderStyle::letterSpacing() const { return fontDescription().letterSpacing(); }
 
 bool RenderStyle::setFontDescription(const FontDescription& v)
 {
     if (inherited->font.fontDescription() != v) {
         inherited.access()->font = Font(v);
         return true;
     }
     return false;
@@ skipping 75 matching lines @@
 
 void RenderStyle::setFontWeight(FontWeight weight)
 {
     FontSelector* currentFontSelector = font().fontSelector();
     FontDescription desc(fontDescription());
     desc.setWeight(weight);
     setFontDescription(desc);
     font().update(currentFontSelector);
 }
 
+void RenderStyle::setTextDecorationsInEffect(TextDecoration decorations)
+{
+    clearAppliedTextDecorations();
+
+    if (decorations & TextDecorationUnderline)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationUnderline));
+    if (decorations & TextDecorationOverline)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationOverline));
+    if (decorations & TextDecorationLineThrough)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationLineThrough));
+}
+
+void RenderStyle::addAppliedTextDecoration(const AppliedTextDecoration& decoration)
+{
+    bool isSimpleUnderline = decoration.isSimple() && decoration.line() == TextDecorationUnderline;

[Julien - ping for review, 2014/05/05 17:45:08]

It's weird that we check for this simple case repeatedly here when the caller
knows all the text-decorations that applies to an element so could easily set
the flag and insert the extra AppliedTextDecoration as needed.

[andersr, 2014/05/06 13:15:29]

I don't know ... is that really weird?

Currently, addAppliedTextDecoration "packs" the simple decoration into a single
bit, and the appliedTextDecorations "unpacks" the simple decoration, as
appropriate. It seems natural to me that this optimization should be an
implementation detail of the setter/getter, and not something utility functions
(applyTextDecorations) and/or callers need to even know about.

+
+    if (!rareInheritedData->appliedTextDecorations && isSimpleUnderline) {
+        // To save memory, we don't use AppliedTextDecoration objects in the
+        // common case of a single solid underline with the current color. Instead,
+        // we set a bit to indicate that a solid underline must be drawn with the
+        // current color.
+        inherited_flags.m_textUnderline = true;
+        return;
+    }
+
+    RefPtr<AppliedTextDecorationList>& list = rareInheritedData.access()->appliedTextDecorations;
+
+    if (!list)
+        list = AppliedTextDecorationList::create();
+    else if (!list->hasOneRef())
+        list = list->copy();

[Julien - ping for review, 2014/05/05 17:45:08]

It's probably me but I have a hard time understanding:
A) Why we need this copy?
B) Why we need such a complex sharing code for a rare property and a Vector that
holds at most 4 elements?

[andersr, 2014/05/06 13:15:29]

A) The same reason we copy DataRef objects? When the applied decorations of a
child changes, it should not apply to the parent. I realize this behavior is
almost identical to DataRef (the difference is that DataRefs must always be
initialized, which we don't want), however, there is already a precedent in
RenderStyle to do it this way in such situations. (CursorList).

I could submit a follow up patch with optional lazy initialization for DataRef.
(Or something like that).

B) How did you arrive at the number '4'? I believe it can hold up to [number of
lines] * [number of styles] effective elements, which currently computes to
'15'.

Example: For each [underline, line-through, overline], you can draw (in order):
[wavy, double, solid, dashed, dotted], and portions of all those decorations
will be visible. Beyond that, I believe some decorations *could* be dropped,
yes, but it makes no sense to optimize such cases.

+
+    if (inherited_flags.m_textUnderline) {
+        inherited_flags.m_textUnderline = false;
+        list->append(AppliedTextDecoration(TextDecorationUnderline, TextDecorationStyleSolid, StyleColor::currentColor()));
+    }
+
+    list->append(decoration);
+}
+
+void RenderStyle::applyTextDecorations()
+{
+    if (textDecoration() == TextDecorationNone)
+        return;
+
+    TextDecorationStyle style = textDecorationStyle();
+    StyleColor styleColor = visitedDependentDecorationStyleColor();
+
+    int decorations = textDecoration();
+
+    if (decorations & TextDecorationUnderline)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationUnderline, style, styleColor));
+    if (decorations & TextDecorationOverline)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationOverline, style, styleColor));
+    if (decorations & TextDecorationLineThrough)
+        addAppliedTextDecoration(AppliedTextDecoration(TextDecorationLineThrough, style, styleColor));
+}
+
+void RenderStyle::clearAppliedTextDecorations()
+{
+    inherited_flags.m_textUnderline = false;
+
+    if (rareInheritedData->appliedTextDecorations)
+        rareInheritedData.access()->appliedTextDecorations = nullptr;
+}
+
 void RenderStyle::getShadowExtent(const ShadowList* shadowList, LayoutUnit &top, LayoutUnit &right, LayoutUnit &bottom, LayoutUnit &left) const
 {
     top = 0;
     right = 0;
     bottom = 0;
     left = 0;
 
     size_t shadowCount = shadowList ? shadowList->shadows().size() : 0;
     for (size_t i = 0; i < shadowCount; ++i) {
         const ShadowData& shadow = shadowList->shadows()[i];
@@ skipping 57 matching lines @@
         const ShadowData& shadow = shadowList->shadows()[i];
         if (shadow.style() == Inset)
             continue;
         float blurAndSpread = shadow.blur() + shadow.spread();
 
         top = min<LayoutUnit>(top, shadow.y() - blurAndSpread);
         bottom = max<LayoutUnit>(bottom, shadow.y() + blurAndSpread);
     }
 }
 
-StyleColor RenderStyle::visitedDependentDecorationColor() const
+StyleColor RenderStyle::visitedDependentDecorationStyleColor() const
 {
-    // Text decoration color fallback is handled in RenderObject::decorationColor.
-    return insideLink() == InsideVisitedLink ? visitedLinkTextDecorationColor() : textDecorationColor();
+    bool visitedLink = insideLink() == InsideVisitedLink;
+
+    StyleColor styleColor = visitedLink ? visitedLinkTextDecorationColor() : textDecorationColor();
+
+    if (!styleColor.isCurrentColor())
+        return styleColor;
+
+    if (textStrokeWidth()) {
+        // Prefer stroke color if possible but not if it's fully transparent.
+        StyleColor textStrokeStyleColor = visitedLink ? visitedLinkTextStrokeColor() : textStrokeColor();
+        if (!textStrokeStyleColor.isCurrentColor() && textStrokeStyleColor.color().alpha())
+            return textStrokeStyleColor;
+    }
+
+    return visitedLink ? visitedLinkTextFillColor() : textFillColor();
+}
+
+Color RenderStyle::visitedDependentDecorationColor() const
+{
+    bool visitedLink = insideLink() == InsideVisitedLink;
+    return visitedDependentDecorationStyleColor().resolve(visitedLink ? visitedLinkColor() : color());
 }
 
 Color RenderStyle::colorIncludingFallback(int colorProperty, bool visitedLink) const
 {
     StyleColor result(StyleColor::currentColor());
     EBorderStyle borderStyle = BNONE;
     switch (colorProperty) {
     case CSSPropertyBackgroundColor:
         result = visitedLink ? visitedLinkBackgroundColor() : backgroundColor();
         break;
@@ skipping 279 matching lines @@
     // right
     radiiSum = radii.topRight().height() + radii.bottomRight().height();
     if (radiiSum > rect.height())
         factor = std::min(rect.height() / radiiSum, factor);
 
     ASSERT(factor <= 1);
     return factor;
 }
 
 } // namespace WebCore