Changeset 108008 in webkit

Timestamp:

Feb 16, 2012 5:40:33 PM (12 years ago)

Author:

mdelaney@apple.com

Message:

ShadowBlur.cpp's cached content matching needs to consider m_layerSize changes
​https://bugs.webkit.org/show_bug.cgi?id=78765

Reviewed by Simon Fraser.

No new tests due to the flaky nature of reproducing the issue.

• platform/graphics/ShadowBlur.cpp:

(WebCore::ScratchBuffer::getScratchBuffer): Make sure to call clearScratchBuffer()
when we create a new ImageBuffer in order to invalidate cached values.
(WebCore::ScratchBuffer::setCachedShadowValues): Roll together matching and setting
of cached values into one method to enforce them being the same.
(WebCore::ScratchBuffer::setCachedInsetShadowValues): Ditto.

Restructure to use new method described above.
(WebCore::ShadowBlur::drawRectShadowWithoutTiling):
(WebCore::ShadowBlur::drawInsetShadowWithoutTiling):
(WebCore::ShadowBlur::drawInsetShadowWithTiling):
(WebCore::ShadowBlur::drawRectShadowWithTiling):
(WebCore::ShadowBlur::beginShadowLayer):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/ShadowBlur.cpp (modified) (13 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-02-16  Matthew Delaney  <mdelaney@apple.com>
+
+        ShadowBlur.cpp's cached content matching needs to consider m_layerSize changes
+        https://bugs.webkit.org/show_bug.cgi?id=78765
+
+        Reviewed by Simon Fraser.
+
+        No new tests due to the flaky nature of reproducing the issue.
+
+        * platform/graphics/ShadowBlur.cpp:
+        (WebCore::ScratchBuffer::getScratchBuffer): Make sure to call clearScratchBuffer()
+        when we create a new ImageBuffer in order to invalidate cached values.
+        (WebCore::ScratchBuffer::setCachedShadowValues): Roll together matching and setting
+        of cached values into one method to enforce them being the same.
+        (WebCore::ScratchBuffer::setCachedInsetShadowValues): Ditto.
+
+        Restructure to use new method described above.
+        (WebCore::ShadowBlur::drawRectShadowWithoutTiling): 
+        (WebCore::ShadowBlur::drawInsetShadowWithoutTiling):
+        (WebCore::ShadowBlur::drawInsetShadowWithTiling):
+        (WebCore::ShadowBlur::drawRectShadowWithTiling):
+        (WebCore::ShadowBlur::beginShadowLayer):
+
 2012-02-16  Dana Jansens  <danakj@chromium.org>
 

trunk/Source/WebCore/platform/graphics/ShadowBlur.cpp

@@ -80 +80 @@
         IntSize roundedSize(roundUpToMultipleOf32(size.width()), roundUpToMultipleOf32(size.height()));
 
+        clearScratchBuffer();
         m_imageBuffer = ImageBuffer::create(roundedSize);
         return m_imageBuffer.get();
     }
 
-    void setLastShadowValues(const FloatSize& radius, const Color& color, ColorSpace colorSpace, const FloatRect& shadowRect, const RoundedRect::Radii& radii)
+    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;
@@ -92 +96 @@
         m_lastShadowRect = shadowRect;
         m_lastRadii = radii;
-    }
-
-    void setLastInsetShadowValues(const FloatSize& radius, const Color& color, ColorSpace colorSpace, const FloatRect& bounds, const FloatRect& shadowRect, const RoundedRect::Radii& 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;
@@ -103 +113 @@
         m_lastShadowRect = shadowRect;
         m_lastRadii = radii;
-    }
-    
-    bool matchesLastShadow(const FloatSize& radius, const Color& color, ColorSpace colorSpace, const FloatRect& shadowRect, const RoundedRect::Radii& radii) const
-    {
-        if (m_lastWasInset)
-            return false;
-        return m_lastRadius == radius && m_lastColor == color && m_lastColorSpace == colorSpace && shadowRect == m_lastShadowRect && radii == m_lastRadii;
-    }
-
-    bool matchesLastInsetShadow(const FloatSize& radius, const Color& color, ColorSpace colorSpace, const FloatRect& bounds, const FloatRect& shadowRect, const RoundedRect::Radii& radii) const
-    {
-        if (!m_lastWasInset)
-            return false;
-        return m_lastRadius == radius && m_lastColor == color && m_lastColorSpace == colorSpace && m_lastInsetBounds == bounds && shadowRect == m_lastShadowRect && radii == m_lastRadii;
+
+        return true;
     }
 
@@ -155 +153 @@
     FloatSize m_lastRadius;
     bool m_lastWasInset;
+    FloatSize m_lastLayerSize;
     
 #if !ASSERT_DISABLED
@@ -544 +543 @@
     FloatRect bufferRelativeShadowedRect = shadowedRect;
     bufferRelativeShadowedRect.move(m_layerContextTranslation);
-    if (!ScratchBuffer::shared().matchesLastShadow(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeShadowedRect, radii)) {
+
+    // Only redraw in the scratch buffer if its cached contents don't match our needs
+    bool redrawNeeded = ScratchBuffer::shared().setCachedShadowValues(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeShadowedRect, radii, m_layerSize);
+    if (redrawNeeded) {
         GraphicsContext* shadowContext = m_layerImage->context();
         GraphicsContextStateSaver stateSaver(*shadowContext);
@@ -561 +563 @@
 
         blurShadowBuffer(expandedIntSize(m_layerSize));
-        
-        ScratchBuffer::shared().setLastShadowValues(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeShadowedRect, radii);
     }
     
@@ -582 +582 @@
     bufferRelativeHoleRect.move(m_layerContextTranslation);
 
-    if (!ScratchBuffer::shared().matchesLastInsetShadow(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeRect, bufferRelativeHoleRect, holeRadii)) {
+    // Only redraw in the scratch buffer if its cached contents don't match our needs
+    bool redrawNeeded = ScratchBuffer::shared().setCachedInsetShadowValues(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeRect, bufferRelativeHoleRect, holeRadii);
+    if (redrawNeeded) {
         GraphicsContext* shadowContext = m_layerImage->context();
         GraphicsContextStateSaver stateSaver(*shadowContext);
@@ -602 +604 @@
 
         blurShadowBuffer(expandedIntSize(m_layerSize));
-
-        ScratchBuffer::shared().setLastInsetShadowValues(m_blurRadius, Color::black, ColorSpaceDeviceRGB, bufferRelativeRect, bufferRelativeHoleRect, holeRadii);
     }
     
@@ -653 +653 @@
     FloatRect templateHole = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
 
-    if (!ScratchBuffer::shared().matchesLastInsetShadow(m_blurRadius, m_color, m_colorSpace, templateBounds, templateHole, radii)) {
+    // Only redraw in the scratch buffer if its cached contents don't match our needs
+    bool redrawNeeded = ScratchBuffer::shared().setCachedInsetShadowValues(m_blurRadius, m_color, m_colorSpace, templateBounds, templateHole, radii);
+    if (redrawNeeded) {
         // Draw shadow into a new ImageBuffer.
         GraphicsContext* shadowContext = m_layerImage->context();
@@ -671 +673 @@
 
         blurAndColorShadowBuffer(templateSize);
-    
-        ScratchBuffer::shared().setLastInsetShadowValues(m_blurRadius, m_color, m_colorSpace, templateBounds, templateHole, radii);
     }
 
@@ -711 +711 @@
     FloatRect templateShadow = FloatRect(edgeSize.width(), edgeSize.height(), templateSize.width() - 2 * edgeSize.width(), templateSize.height() - 2 * edgeSize.height());
 
-    if (!ScratchBuffer::shared().matchesLastShadow(m_blurRadius, m_color, m_colorSpace, templateShadow, radii)) {
+    // Only redraw in the scratch buffer if its cached contents don't match our needs
+    bool redrawNeeded = ScratchBuffer::shared().setCachedShadowValues(m_blurRadius, m_color, m_colorSpace, templateShadow, radii, m_layerSize);
+    if (redrawNeeded) {
         // Draw shadow into the ImageBuffer.
         GraphicsContext* shadowContext = m_layerImage->context();
@@ -728 +730 @@
 
         blurAndColorShadowBuffer(templateSize);
-
-        ScratchBuffer::shared().setLastShadowValues(m_blurRadius, m_color, m_colorSpace, templateShadow, radii);
     }
 
@@ -855 +855 @@
     // We reset the scratch buffer values here, because the buffer will no longer contain
     // data from any previous rectangle or inset shadows drawn via the tiling path.
-    ScratchBuffer::shared().setLastShadowValues(FloatSize(), Color::black, ColorSpaceDeviceRGB, IntRect(), RoundedRect::Radii());
+    ScratchBuffer::shared().setCachedShadowValues(FloatSize(), Color::black, ColorSpaceDeviceRGB, IntRect(), RoundedRect::Radii(), m_layerSize);
     m_layerImage = ScratchBuffer::shared().getScratchBuffer(layerRect.size());