- Revision
- 137038
- Author
- [email protected]
- Date
- 2012-12-08 09:42:14 -0800 (Sat, 08 Dec 2012)
Log Message
Merge 132856 - feImage should not be allowed to self reference
https://bugs.webkit.org/show_bug.cgi?id=94652
Reviewed by Eric Seidel.
Source/WebCore:
Add cycle detection for SVG filter application, and also fix a problem
with graphics context restore when filters are applied. This also
converts the flags in FilterData to a state tracking system, as the
number of flags was getting messy and only one flag is valid at any given time.
Test: svg/filters/feImage-self-and-other-referencing.html
* rendering/svg/RenderSVGResourceFilter.cpp: Convert to new FilterData
state management and enable cycle detection.
(WebCore):
(WebCore::RenderSVGResourceFilter::removeClientFromCache): Change isBuilt and markedForRemoval flags to state enums.
(WebCore::RenderSVGResourceFilter::applyResource): Change flags to state enums and detect cycles.
(WebCore::RenderSVGResourceFilter::postApplyResource): Change flags to state and add handling
for the various states.
(WebCore::RenderSVGResourceFilter::primitiveAttributeChanged): Change isBuilt flag to state enums.
* rendering/svg/RenderSVGResourceFilter.h:
(WebCore::FilterData::FilterData):
(FilterData): Convert to a state tracking system.
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::paintReplaced): Add a block around the
SVGRenderingContext so that it applies the filter and reverts the
context before the calling method restores the context.
LayoutTests:
Additional test case for situations when the filter is applied to multiple objects that it also references.
* svg/filters/feImage-self-and-other-referencing-expected.html: Added.
* svg/filters/feImage-self-and-other-referencing.html: Added.
Conflicts:
Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp
Source/WebCore/rendering/svg/RenderSVGResourceFilter.h
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-1.10/LayoutTests/ChangeLog (137037 => 137038)
--- releases/WebKitGTK/webkit-1.10/LayoutTests/ChangeLog 2012-12-08 17:41:43 UTC (rev 137037)
+++ releases/WebKitGTK/webkit-1.10/LayoutTests/ChangeLog 2012-12-08 17:42:14 UTC (rev 137038)
@@ -1,3 +1,15 @@
+2012-10-25 Stephen Chenney <[email protected]>
+
+ feImage should not be allowed to self reference
+ https://bugs.webkit.org/show_bug.cgi?id=94652
+
+ Reviewed by Eric Seidel.
+
+ Additional test case for situations when the filter is applied to multiple objects that it also references.
+
+ * svg/filters/feImage-self-and-other-referencing-expected.html: Added.
+ * svg/filters/feImage-self-and-other-referencing.html: Added.
+
2012-10-16 Stephen Chenney <[email protected]>
An feImage that tries to render itself should be stopped
Added: releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html (0 => 137038)
--- releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html (rev 0)
+++ releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html 2012-12-08 17:42:14 UTC (rev 137038)
@@ -0,0 +1,6 @@
+<html>
+ <body>
+ <p>PASS if no crash</p>
+ </body>
+</html>
+
Added: releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing.html (0 => 137038)
--- releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing.html (rev 0)
+++ releases/WebKitGTK/webkit-1.10/LayoutTests/svg/filters/feImage-self-and-other-referencing.html 2012-12-08 17:42:14 UTC (rev 137038)
@@ -0,0 +1,21 @@
+<html>
+ <head>
+ <script>
+ _onload_ = function() {
+ document.getElementById("svgRoot").setAttribute('filter', 'url(#imageFilter)')
+ }
+ </script>
+ </head>
+ <body>
+ <p>PASS if no crash</p>
+ <svg width="10" height="10" id="svgRoot" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <filter id="imageFilter">
+ <feImage id="feImage" xlink:href=""
+ <feOffset dx="50" dy="50" />
+ </filter>
+ <g filter="url(#imageFilter)">
+ <rect x="0" y="0" width="50" height="50" fill="green" />
+ </g>
+ </svg>
+ </body>
+</html>
Modified: releases/WebKitGTK/webkit-1.10/Source/WebCore/ChangeLog (137037 => 137038)
--- releases/WebKitGTK/webkit-1.10/Source/WebCore/ChangeLog 2012-12-08 17:41:43 UTC (rev 137037)
+++ releases/WebKitGTK/webkit-1.10/Source/WebCore/ChangeLog 2012-12-08 17:42:14 UTC (rev 137038)
@@ -1,3 +1,33 @@
+2012-10-25 Stephen Chenney <[email protected]>
+
+ feImage should not be allowed to self reference
+ https://bugs.webkit.org/show_bug.cgi?id=94652
+
+ Reviewed by Eric Seidel.
+
+ Add cycle detection for SVG filter application, and also fix a problem
+ with graphics context restore when filters are applied. This also
+ converts the flags in FilterData to a state tracking system, as the
+ number of flags was getting messy and only one flag is valid at any given time.
+
+ Test: svg/filters/feImage-self-and-other-referencing.html
+
+ * rendering/svg/RenderSVGResourceFilter.cpp: Convert to new FilterData
+ state management and enable cycle detection.
+ (WebCore):
+ (WebCore::RenderSVGResourceFilter::removeClientFromCache): Change isBuilt and markedForRemoval flags to state enums.
+ (WebCore::RenderSVGResourceFilter::applyResource): Change flags to state enums and detect cycles.
+ (WebCore::RenderSVGResourceFilter::postApplyResource): Change flags to state and add handling
+ for the various states.
+ (WebCore::RenderSVGResourceFilter::primitiveAttributeChanged): Change isBuilt flag to state enums.
+ * rendering/svg/RenderSVGResourceFilter.h:
+ (WebCore::FilterData::FilterData):
+ (FilterData): Convert to a state tracking system.
+ * rendering/svg/RenderSVGRoot.cpp:
+ (WebCore::RenderSVGRoot::paintReplaced): Add a block around the
+ SVGRenderingContext so that it applies the filter and reverts the
+ context before the calling method restores the context.
+
2012-10-16 Stephen Chenney <[email protected]>
An feImage that tries to render itself should be stopped
https://bugs.webkit.org/show_bug.cgi?id=94652
Modified: releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (137037 => 137038)
--- releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2012-12-08 17:41:43 UTC (rev 137037)
+++ releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2012-12-08 17:42:14 UTC (rev 137038)
@@ -57,25 +57,6 @@
namespace WebCore {
-class ApplyingFilterEffectGuard {
-public:
- ApplyingFilterEffectGuard(FilterData* data)
- : m_filterData(data)
- {
- // The guard must be constructed when the filter is not applying.
- ASSERT(!m_filterData->isApplying);
- m_filterData->isApplying = true;
- }
-
- ~ApplyingFilterEffectGuard()
- {
- ASSERT(m_filterData->isApplying);
- m_filterData->isApplying = false;
- }
-
- FilterData* m_filterData;
-};
-
RenderSVGResourceType RenderSVGResourceFilter::s_resourceType = FilterResourceType;
RenderSVGResourceFilter::RenderSVGResourceFilter(SVGFilterElement* node)
@@ -108,7 +89,7 @@
if (FilterData* filterData = m_filter.get(client)) {
if (filterData->savedContext)
- filterData->markedForRemoval = true;
+ filterData->state = FilterData::MarkedForRemoval;
else
delete m_filter.take(client);
}
@@ -168,14 +149,11 @@
ASSERT(context);
ASSERT_UNUSED(resourceMode, resourceMode == ApplyToDefaultMode);
- // Returning false here, to avoid drawings onto the context. We just want to
- // draw the stored filter output, not the unfiltered object as well.
if (m_filter.contains(object)) {
FilterData* filterData = m_filter.get(object);
- if (filterData->isBuilt || filterData->isApplying)
- return false;
-
- delete m_filter.take(object); // Oops, have to rebuild, go through normal code path
+ if (filterData->state == FilterData::PaintingSource || filterData->state == FilterData::Applying)
+ filterData->state = FilterData::CycleDetected;
+ return false; // Already built, or we're in a cycle, or we're marked for removal. Regardless, just do nothing more now.
}
OwnPtr<FilterData> filterData(adoptPtr(new FilterData));
@@ -292,17 +270,21 @@
if (!filterData)
return;
- if (filterData->markedForRemoval) {
+ switch (filterData->state) {
+ case FilterData::MarkedForRemoval:
delete m_filter.take(object);
return;
- }
- // We have a cycle if we are already applying the data.
- // This can occur due to FeImage referencing a source that makes use of the FEImage itself.
- if (filterData->isApplying)
+ case FilterData::CycleDetected:
+ case FilterData::Applying:
+ // We have a cycle if we are already applying the data.
+ // This can occur due to FeImage referencing a source that makes use of the FEImage itself.
+ // This is the first place we've hit the cycle, so set the state back to PaintingSource so the return stack
+ // will continue correctly.
+ filterData->state = FilterData::PaintingSource;
return;
- if (!filterData->isBuilt) {
+ case FilterData::PaintingSource:
if (!filterData->savedContext) {
removeClientFromCache(object);
return;
@@ -310,21 +292,23 @@
context = filterData->savedContext;
filterData->savedContext = 0;
+ break;
+
+ case FilterData::Built: { } // Empty
}
- ApplyingFilterEffectGuard isApplyingGuard(filterData);
-
FilterEffect* lastEffect = filterData->builder->lastEffect();
if (lastEffect && !filterData->boundaries.isEmpty() && !lastEffect->filterPrimitiveSubregion().isEmpty()) {
// This is the real filtering of the object. It just needs to be called on the
// initial filtering process. We just take the stored filter result on a
// second drawing.
- if (!filterData->isBuilt)
+ if (filterData->state != FilterData::Built)
filterData->filter->setSourceImage(filterData->sourceGraphicBuffer.release());
- // Always true if filterData is just built (filterData->isBuilt is false).
+ // Always true if filterData is just built (filterData->state == FilterData::Built).
if (!lastEffect->hasResult()) {
+ filterData->state = FilterData::Applying;
lastEffect->apply();
lastEffect->correctFilterResultIfNeeded();
#if !USE(CG)
@@ -333,7 +317,7 @@
resultImage->transformColorSpace(lastEffect->colorSpace(), ColorSpaceDeviceRGB);
#endif
}
- filterData->isBuilt = true;
+ filterData->state = FilterData::Built;
ImageBuffer* resultImage = lastEffect->asImageBuffer();
if (resultImage) {
@@ -365,7 +349,7 @@
for (; it != end; ++it) {
FilterData* filterData = it->second;
- if (!filterData->isBuilt)
+ if (filterData->state != FilterData::Built)
continue;
SVGFilterBuilder* builder = filterData->builder.get();
Modified: releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h (137037 => 137038)
--- releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2012-12-08 17:41:43 UTC (rev 137037)
+++ releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2012-12-08 17:42:14 UTC (rev 137038)
@@ -40,11 +40,12 @@
namespace WebCore {
struct FilterData {
+public:
+ enum FilterDataState { PaintingSource, Applying, Built, CycleDetected, MarkedForRemoval };
+
FilterData()
: savedContext(0)
- , isBuilt(false)
- , isApplying(false)
- , markedForRemoval(false)
+ , state(PaintingSource)
{
}
@@ -55,9 +56,7 @@
AffineTransform shearFreeAbsoluteTransform;
FloatRect boundaries;
FloatSize scale;
- bool isBuilt : 1;
- bool isApplying : 1;
- bool markedForRemoval : 1;
+ FilterDataState state;
};
class GraphicsContext;
Modified: releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (137037 => 137038)
--- releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2012-12-08 17:41:43 UTC (rev 137037)
+++ releases/WebKitGTK/webkit-1.10/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2012-12-08 17:42:14 UTC (rev 137038)
@@ -298,16 +298,20 @@
IntPoint adjustedPaintOffset = roundedIntPoint(paintOffset);
childPaintInfo.applyTransform(AffineTransform::translation(adjustedPaintOffset.x() - x(), adjustedPaintOffset.y() - y()) * localToParentTransform());
- SVGRenderingContext renderingContext;
- bool continueRendering = true;
- if (childPaintInfo.phase == PaintPhaseForeground) {
- renderingContext.prepareToRenderSVGContent(this, childPaintInfo);
- continueRendering = renderingContext.isRenderingPrepared();
+ // SVGRenderingContext must be destroyed before we restore the childPaintInfo.context, because a filter may have
+ // changed the context and it is only reverted when the SVGRenderingContext destructor finishes applying the filter.
+ {
+ SVGRenderingContext renderingContext;
+ bool continueRendering = true;
+ if (childPaintInfo.phase == PaintPhaseForeground) {
+ renderingContext.prepareToRenderSVGContent(this, childPaintInfo);
+ continueRendering = renderingContext.isRenderingPrepared();
+ }
+
+ if (continueRendering)
+ RenderBox::paint(childPaintInfo, LayoutPoint());
}
- if (continueRendering)
- RenderBox::paint(childPaintInfo, LayoutPoint());
-
childPaintInfo.context->restore();
}
