[webkit-reviews] review denied: [Bug 124394] -webkit-text-decoration-skip: ink is slow : [Attachment 217083] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 124394: -webkit-text-decoration-skip: ink is slow
https://bugs.webkit.org/show_bug.cgi?id=124394

Attachment 217083: Patch
https://bugs.webkit.org/attachment.cgi?id=217083&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=217083&action=review


> Source/WebCore/platform/graphics/ScratchBuffer.h:107
> +    bool setCachedShadowValues(const FloatSize& radius, const Color& color,
ColorSpace colorSpace, const FloatRect& shadowRect, const RoundedRect::Radii&
radii, const FloatSize& layerSize)
> +    {
> +	   if (!m_lastWasInset && m_lastRadius == radius && m_lastColor ==
color && m_lastColorSpace == colorSpace && m_lastShadowRect == shadowRect && 
m_lastRadii == radii && m_lastLayerSize == layerSize)
> +	       return false;
> +
> +	   m_lastWasInset = false;
> +	   m_lastRadius = radius;
> +	   m_lastColor = color;
> +	   m_lastColorSpace = colorSpace;
> +	   m_lastShadowRect = shadowRect;
> +	   m_lastRadii = radii;
> +	   m_lastLayerSize = layerSize;
> +
> +	   return true;
> +    }
> +
> +    bool setCachedInsetShadowValues(const FloatSize& radius, const Color&
color, ColorSpace colorSpace, const FloatRect& bounds, const FloatRect&
shadowRect, const RoundedRect::Radii& radii)
> +    {
> +	   if (m_lastWasInset && m_lastRadius == radius && m_lastColor == color
&& m_lastColorSpace == colorSpace && m_lastInsetBounds == bounds && shadowRect
== m_lastShadowRect && radii == m_lastRadii)
> +	       return false;
> +
> +	   m_lastWasInset = true;
> +	   m_lastInsetBounds = bounds;
> +	   m_lastRadius = radius;
> +	   m_lastColor = color;
> +	   m_lastColorSpace = colorSpace;
> +	   m_lastShadowRect = shadowRect;
> +	   m_lastRadii = radii;
> +
> +	   return true;
> +    }

This class shouldn't have shadow-related stuff in it.

> Source/WebCore/rendering/InlineTextBox.cpp:1081
> +		   FloatRect imageBufferRectInUserSpace =
FloatRect(enclosingUnderlineRect.location(),
> +		       FloatSize(enclosingUnderlineRect.size().width() *
xMagnification,
> +			   enclosingUnderlineRect.size().height() *
yMagnification));

Would be nice to push this complexity into some buffer class that under the
hood can use larger buffers.