From 2c71f77297f2db6cfb84c39f92db017a3769137d Mon Sep 17 00:00:00 2001 From: Ojan Vafai Date: Thu, 4 Dec 2014 12:15:39 -0800 Subject: [PATCH] Get rid of background-clip:text and PaintBehaviorForceBlackText. http://blog.ericzhang.com/punch-through-text-masks-with-css-and-html5/ shows that you can achieve the similar effects using canvas. This doesn't need to be built into the core engine. It's OK for this to require more than a line of CSS to achieve. Unfortunately, the test included this this patch doesn't actually show that I didn't break anything because background-image is broken. I got the test from https://chromium.googlesource.com/chromium/blink/+/47e0d126e8d459b10176e3a91e47cf62ce0953a5%5E%21/#F4 R=eseidel@chromium.org Review URL: https://codereview.chromium.org/756183004 --- engine/core/css/CSSPrimitiveValueMappings.h | 5 -- engine/core/rendering/EllipsisBox.cpp | 2 +- engine/core/rendering/InlineTextBox.cpp | 49 +++++-------------- engine/core/rendering/PaintInfo.h | 2 - engine/core/rendering/PaintPhase.h | 2 +- .../core/rendering/RenderBoxModelObject.cpp | 38 -------------- engine/core/rendering/RenderLayer.cpp | 2 - engine/core/rendering/RenderView.cpp | 3 -- engine/core/rendering/style/FillLayer.cpp | 4 +- .../rendering/style/RenderStyleConstants.h | 2 +- engine/platform/graphics/Color.cpp | 8 --- engine/platform/graphics/Color.h | 2 - ...ound-opaque-clipped-gradients-expected.txt | 10 ++++ .../background-opaque-clipped-gradients.sky | 17 +++++++ 14 files changed, 42 insertions(+), 104 deletions(-) create mode 100644 tests/clipping/background-opaque-clipped-gradients-expected.txt create mode 100644 tests/clipping/background-opaque-clipped-gradients.sky diff --git a/engine/core/css/CSSPrimitiveValueMappings.h b/engine/core/css/CSSPrimitiveValueMappings.h index c5ddf956737..34c26fdd8ec 100644 --- a/engine/core/css/CSSPrimitiveValueMappings.h +++ b/engine/core/css/CSSPrimitiveValueMappings.h @@ -348,9 +348,6 @@ template<> inline CSSPrimitiveValue::CSSPrimitiveValue(EFillBox e) case ContentFillBox: m_value.valueID = CSSValueContentBox; break; - case TextFillBox: - m_value.valueID = CSSValueText; - break; } } @@ -367,8 +364,6 @@ template<> inline CSSPrimitiveValue::operator EFillBox() const case CSSValueContent: case CSSValueContentBox: return ContentFillBox; - case CSSValueText: - return TextFillBox; default: break; } diff --git a/engine/core/rendering/EllipsisBox.cpp b/engine/core/rendering/EllipsisBox.cpp index 86e63d1e8d3..47147a96921 100644 --- a/engine/core/rendering/EllipsisBox.cpp +++ b/engine/core/rendering/EllipsisBox.cpp @@ -53,7 +53,7 @@ void EllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, La paintSelection(context, boxOrigin, style, font); // Select the correct color for painting the text. - Color foreground = paintInfo.forceBlackText() ? Color::black : renderer().selectionForegroundColor(); + Color foreground = renderer().selectionForegroundColor(); if (foreground != styleTextColor) context->setFillColor(foreground); } diff --git a/engine/core/rendering/InlineTextBox.cpp b/engine/core/rendering/InlineTextBox.cpp index 6727f349290..98144a1838a 100644 --- a/engine/core/rendering/InlineTextBox.cpp +++ b/engine/core/rendering/InlineTextBox.cpp @@ -309,13 +309,6 @@ float InlineTextBox::placeEllipsisBox(bool flowIsLTR, float visibleLeftEdge, flo return -1; } -static Color textColorForWhiteBackground(Color textColor) -{ - int distanceFromWhite = differenceSquared(textColor, Color::white); - // semi-arbitrarily chose 65025 (255^2) value here after a few tests; - return distanceFromWhite > 65025 ? textColor : textColor.dark(); -} - bool InlineTextBox::isLineBreak() const { return renderer().style()->preserveNewline() && len() == 1 && (*renderer().text().impl())[start()] == '\n'; @@ -366,44 +359,24 @@ struct TextPaintingStyle { bool operator!=(const TextPaintingStyle& other) { return !(*this == other); } }; -TextPaintingStyle textPaintingStyle(RenderText& renderer, RenderStyle* style, bool forceBlackText) +TextPaintingStyle textPaintingStyle(RenderText& renderer, RenderStyle* style) { TextPaintingStyle textStyle; - - if (forceBlackText) { - textStyle.fillColor = Color::black; - textStyle.strokeColor = Color::black; - textStyle.emphasisMarkColor = Color::black; - textStyle.strokeWidth = style->textStrokeWidth(); - textStyle.shadow = 0; - } else { - textStyle.fillColor = renderer.resolveColor(style, CSSPropertyWebkitTextFillColor); - textStyle.strokeColor = renderer.resolveColor(style, CSSPropertyWebkitTextStrokeColor); - textStyle.emphasisMarkColor = renderer.resolveColor(style, CSSPropertyWebkitTextEmphasisColor); - textStyle.strokeWidth = style->textStrokeWidth(); - textStyle.shadow = style->textShadow(); - - // Adjust text color when printing with a white background. - bool forceBackgroundToWhite = false; - if (forceBackgroundToWhite) { - textStyle.fillColor = textColorForWhiteBackground(textStyle.fillColor); - textStyle.strokeColor = textColorForWhiteBackground(textStyle.strokeColor); - textStyle.emphasisMarkColor = textColorForWhiteBackground(textStyle.emphasisMarkColor); - } - } - + textStyle.fillColor = renderer.resolveColor(style, CSSPropertyWebkitTextFillColor); + textStyle.strokeColor = renderer.resolveColor(style, CSSPropertyWebkitTextStrokeColor); + textStyle.emphasisMarkColor = renderer.resolveColor(style, CSSPropertyWebkitTextEmphasisColor); + textStyle.strokeWidth = style->textStrokeWidth(); + textStyle.shadow = style->textShadow(); return textStyle; } -TextPaintingStyle selectionPaintingStyle(RenderText& renderer, bool haveSelection, bool forceBlackText, const TextPaintingStyle& textStyle) +TextPaintingStyle selectionPaintingStyle(RenderText& renderer, bool haveSelection, const TextPaintingStyle& textStyle) { TextPaintingStyle selectionStyle = textStyle; if (haveSelection) { - if (!forceBlackText) { - selectionStyle.fillColor = renderer.selectionForegroundColor(); - selectionStyle.emphasisMarkColor = renderer.selectionEmphasisMarkColor(); - } + selectionStyle.fillColor = renderer.selectionForegroundColor(); + selectionStyle.emphasisMarkColor = renderer.selectionEmphasisMarkColor(); } return selectionStyle; @@ -564,8 +537,8 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, bool useCustomUnderlines = containsComposition && renderer().frame()->inputMethodController().compositionUsesCustomUnderlines(); // Determine text colors. - TextPaintingStyle textStyle = textPaintingStyle(renderer(), styleToUse, paintInfo.forceBlackText()); - TextPaintingStyle selectionStyle = selectionPaintingStyle(renderer(), haveSelection, paintInfo.forceBlackText(), textStyle); + TextPaintingStyle textStyle = textPaintingStyle(renderer(), styleToUse); + TextPaintingStyle selectionStyle = selectionPaintingStyle(renderer(), haveSelection, textStyle); bool paintSelectedTextOnly = (paintInfo.phase == PaintPhaseSelection); bool paintSelectedTextSeparately = !paintSelectedTextOnly && textStyle != selectionStyle; diff --git a/engine/core/rendering/PaintInfo.h b/engine/core/rendering/PaintInfo.h index cf606c35261..8bec7d7a52e 100644 --- a/engine/core/rendering/PaintInfo.h +++ b/engine/core/rendering/PaintInfo.h @@ -76,8 +76,6 @@ struct PaintInfo { return !paintingRoot || paintingRoot == renderer; } - bool forceBlackText() const { return paintBehavior & PaintBehaviorForceBlackText; } - void applyTransform(const AffineTransform& localToAncestorTransform, bool identityStatusUnknown = true) { if (identityStatusUnknown && localToAncestorTransform.isIdentity()) diff --git a/engine/core/rendering/PaintPhase.h b/engine/core/rendering/PaintPhase.h index c9bc4625e66..92e16667b45 100644 --- a/engine/core/rendering/PaintPhase.h +++ b/engine/core/rendering/PaintPhase.h @@ -51,9 +51,9 @@ enum PaintPhase { PaintPhaseClippingMask, }; +// FIXME(sky): Remove this enum now that it's unused. enum PaintBehaviorFlags { PaintBehaviorNormal = 0, - PaintBehaviorForceBlackText = 1 << 0, }; typedef unsigned PaintBehavior; diff --git a/engine/core/rendering/RenderBoxModelObject.cpp b/engine/core/rendering/RenderBoxModelObject.cpp index 3ae885bc789..44d7c21eada 100644 --- a/engine/core/rendering/RenderBoxModelObject.cpp +++ b/engine/core/rendering/RenderBoxModelObject.cpp @@ -480,20 +480,6 @@ void RenderBoxModelObject::paintFillLayerExtended(const PaintInfo& paintInfo, co break; } - case TextFillBox: { - // First figure out how big the mask has to be. It should be no bigger than what we need - // to actually render, so we should intersect the dirty rect with the border box of the background. - maskRect = pixelSnappedIntRect(rect); - maskRect.intersect(paintInfo.rect); - - // We draw the background into a separate layer, to be later masked with yet another layer - // holding the text content. - backgroundClipStateSaver.save(); - context->clip(maskRect); - context->beginTransparencyLayer(1); - - break; - } case BorderFillBox: break; default: @@ -544,30 +530,6 @@ void RenderBoxModelObject::paintFillLayerExtended(const PaintInfo& paintInfo, co context->setImageInterpolationQuality(previousInterpolationQuality); } } - - if (bgLayer.clip() == TextFillBox) { - // Create the text mask layer. - context->setCompositeOperation(CompositeDestinationIn); - context->beginTransparencyLayer(1); - - // FIXME: Workaround for https://code.google.com/p/skia/issues/detail?id=1291. - context->clearRect(maskRect); - - // Now draw the text into the mask. We do this by painting using a special paint phase that signals to - // InlineTextBoxes that they should just add their contents to the clip. - PaintInfo info(context, maskRect, PaintPhaseTextClip, PaintBehaviorForceBlackText, 0); - context->setCompositeOperation(CompositeSourceOver); - if (box) { - RootInlineBox& root = box->root(); - box->paint(info, LayoutPoint(scrolledPaintRect.x() - box->x(), scrolledPaintRect.y() - box->y()), root.lineTop(), root.lineBottom()); - } else { - LayoutSize localOffset = isBox() ? toRenderBox(this)->locationOffset() : LayoutSize(); - paint(info, scrolledPaintRect.location() - localOffset); - } - - context->endLayer(); - context->endLayer(); - } } static inline int resolveWidthForRatio(int height, const FloatSize& intrinsicRatio) diff --git a/engine/core/rendering/RenderLayer.cpp b/engine/core/rendering/RenderLayer.cpp index 9ecbc237deb..bd7465684a4 100644 --- a/engine/core/rendering/RenderLayer.cpp +++ b/engine/core/rendering/RenderLayer.cpp @@ -1109,8 +1109,6 @@ void RenderLayer::paintLayerContents(GraphicsContext* context, const LayerPainti if (localPaintingInfo.paintingRoot && !renderer()->isDescendantOf(localPaintingInfo.paintingRoot)) paintingRootForRenderer = localPaintingInfo.paintingRoot; - ASSERT(!(localPaintingInfo.paintBehavior & PaintBehaviorForceBlackText)); - // FIXME(sky): Get rid of PaintBehavior argument now that it's always Normal. PaintBehavior paintBehavior = PaintBehaviorNormal; diff --git a/engine/core/rendering/RenderView.cpp b/engine/core/rendering/RenderView.cpp index 5d43abf06ef..a4ba6028a98 100644 --- a/engine/core/rendering/RenderView.cpp +++ b/engine/core/rendering/RenderView.cpp @@ -241,9 +241,6 @@ static inline bool rendererObscuresBackground(RenderBox* rootBox) || style->hasTransform()) return false; - if (rootBox->style()->backgroundClip() == TextFillBox) - return false; - return true; } diff --git a/engine/core/rendering/style/FillLayer.cpp b/engine/core/rendering/style/FillLayer.cpp index 5f1b37b9590..fd565ac4b51 100644 --- a/engine/core/rendering/style/FillLayer.cpp +++ b/engine/core/rendering/style/FillLayer.cpp @@ -312,9 +312,7 @@ static EFillBox clipMax(EFillBox clipA, EFillBox clipB) return BorderFillBox; if (clipA == PaddingFillBox || clipB == PaddingFillBox) return PaddingFillBox; - if (clipA == ContentFillBox || clipB == ContentFillBox) - return ContentFillBox; - return TextFillBox; + return ContentFillBox; } void FillLayer::computeClipMax() const diff --git a/engine/core/rendering/style/RenderStyleConstants.h b/engine/core/rendering/style/RenderStyleConstants.h index be73df87ffc..3f69b68d0be 100644 --- a/engine/core/rendering/style/RenderStyleConstants.h +++ b/engine/core/rendering/style/RenderStyleConstants.h @@ -95,7 +95,7 @@ enum EFillAttachment { }; enum EFillBox { - BorderFillBox, PaddingFillBox, ContentFillBox, TextFillBox + BorderFillBox, PaddingFillBox, ContentFillBox }; enum EFillRepeat { diff --git a/engine/platform/graphics/Color.cpp b/engine/platform/graphics/Color.cpp index 44221a850ed..2e2fde5a0eb 100644 --- a/engine/platform/graphics/Color.cpp +++ b/engine/platform/graphics/Color.cpp @@ -167,14 +167,6 @@ bool Color::parseHexColor(const String& name, RGBA32& rgb) return parseHexColor(name.characters16(), name.length(), rgb); } -int differenceSquared(const Color& c1, const Color& c2) -{ - int dR = c1.red() - c2.red(); - int dG = c1.green() - c2.green(); - int dB = c1.blue() - c2.blue(); - return dR * dR + dG * dG + dB * dB; -} - bool Color::setFromString(const String& name) { if (name[0] != '#') diff --git a/engine/platform/graphics/Color.h b/engine/platform/graphics/Color.h index 9fdffac7fef..8145ad9bc33 100644 --- a/engine/platform/graphics/Color.h +++ b/engine/platform/graphics/Color.h @@ -45,8 +45,6 @@ PLATFORM_EXPORT RGBA32 makeRGBA32FromFloats(float r, float g, float b, float a); PLATFORM_EXPORT RGBA32 makeRGBAFromHSLA(double h, double s, double l, double a); PLATFORM_EXPORT RGBA32 makeRGBAFromCMYKA(float c, float m, float y, float k, float a); -PLATFORM_EXPORT int differenceSquared(const Color&, const Color&); - inline int redChannel(RGBA32 color) { return (color >> 16) & 0xFF; } inline int greenChannel(RGBA32 color) { return (color >> 8) & 0xFF; } inline int blueChannel(RGBA32 color) { return color & 0xFF; } diff --git a/tests/clipping/background-opaque-clipped-gradients-expected.txt b/tests/clipping/background-opaque-clipped-gradients-expected.txt new file mode 100644 index 00000000000..6ab563f5b6d --- /dev/null +++ b/tests/clipping/background-opaque-clipped-gradients-expected.txt @@ -0,0 +1,10 @@ +layer at (0,0) size 800x600 + RenderView {#document} at (0,0) size 800x600 +layer at (0,0) size 800x238 + RenderBlock {sky} at (0,0) size 800x238 + RenderBlock (anonymous) at (0,0) size 800x38 + RenderText {#text} at (0,0) size 778x38 + text run at (0,0) width 778: "Test passes if the image below shows nested green, blue and yellow squares with a dotted black" + text run at (0,19) width 59: "border." + RenderBlock {div} at (0,38) size 200x200 [border: (25px dotted #000000)] + diff --git a/tests/clipping/background-opaque-clipped-gradients.sky b/tests/clipping/background-opaque-clipped-gradients.sky new file mode 100644 index 00000000000..d14870f2bc4 --- /dev/null +++ b/tests/clipping/background-opaque-clipped-gradients.sky @@ -0,0 +1,17 @@ + + + + +Test passes if the image below shows nested green, blue and yellow squares with a dotted black border. +
+