Diff
Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (148326 => 148327)
--- branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 00:49:23 UTC (rev 148326)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 00:55:49 UTC (rev 148327)
@@ -1,3 +1,19 @@
+2013-04-12 Tim Horton <timothy_hor...@apple.com>
+
+ Merge r132856
+
+ 2012-10-25 Stephen Chenney <schen...@chromium.org>
+
+ 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.
+
2013-04-12 Tim Horton <timothy_hor...@apple.com>
Merge r131488
Added: branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html (0 => 148327)
--- branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing-expected.html 2013-04-13 00:55:49 UTC (rev 148327)
@@ -0,0 +1,6 @@
+<html>
+ <body>
+ <p>PASS if no crash</p>
+ </body>
+</html>
+
Added: branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing.html (0 => 148327)
--- branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing.html (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-and-other-referencing.html 2013-04-13 00:55:49 UTC (rev 148327)
@@ -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: branches/safari-536.30-branch/Source/WebCore/ChangeLog (148326 => 148327)
--- branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 00:49:23 UTC (rev 148326)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 00:55:49 UTC (rev 148327)
@@ -1,5 +1,39 @@
2013-04-12 Tim Horton <timothy_hor...@apple.com>
+ Merge r132856
+
+ 2012-10-25 Stephen Chenney <schen...@chromium.org>
+
+ 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.
+
+2013-04-12 Tim Horton <timothy_hor...@apple.com>
+
Merge r131488
2012-10-16 Stephen Chenney <schen...@chromium.org>
Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (148326 => 148327)
--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2013-04-13 00:49:23 UTC (rev 148326)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2013-04-13 00:55:49 UTC (rev 148327)
@@ -55,25 +55,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)
@@ -106,7 +87,7 @@
if (FilterData* filterData = m_filter.get(client)) {
if (filterData->savedContext)
- filterData->markedForRemoval = true;
+ filterData->state = FilterData::MarkedForRemoval;
else
delete m_filter.take(client);
}
@@ -164,14 +145,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));
@@ -288,17 +266,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 @@
if (filterData->sourceGraphicBuffer)
filterData->sourceGraphicBuffer->transformColorSpace(ColorSpaceDeviceRGB, ColorSpaceLinearRGB);
#endif
+ 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(ColorSpaceLinearRGB, ColorSpaceDeviceRGB);
#endif
}
- filterData->isBuilt = true;
+ filterData->state = FilterData::Built;
ImageBuffer* resultImage = lastEffect->asImageBuffer();
if (resultImage) {
@@ -366,7 +350,7 @@
for (; it != end; ++it) {
FilterData* filterData = it->second;
- if (!filterData->isBuilt)
+ if (filterData->state != FilterData::Built)
continue;
SVGFilterBuilder* builder = filterData->builder.get();
Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h (148326 => 148327)
--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2013-04-13 00:49:23 UTC (rev 148326)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2013-04-13 00:55:49 UTC (rev 148327)
@@ -40,11 +40,11 @@
namespace WebCore {
struct FilterData {
+ enum FilterDataState { PaintingSource, Applying, Built, CycleDetected, MarkedForRemoval };
+
FilterData()
: savedContext(0)
- , isBuilt(false)
- , isApplying(false)
- , markedForRemoval(false)
+ , state(PaintingSource)
{
}
@@ -55,9 +55,7 @@
AffineTransform shearFreeAbsoluteTransform;
FloatRect boundaries;
FloatSize scale;
- bool isBuilt : 1;
- bool isApplying : 1;
- bool markedForRemoval : 1;
+ FilterDataState state;
};
class GraphicsContext;
Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (148326 => 148327)
--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2013-04-13 00:49:23 UTC (rev 148326)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2013-04-13 00:55:49 UTC (rev 148327)
@@ -293,16 +293,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();
}
