Bug 181974 - feFlood doesn't work when filter resolution is reduced
Summary: feFlood doesn't work when filter resolution is reduced
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-22 22:23 PST by Simon Fraser (smfr)
Modified: 2018-02-01 21:35 PST (History)
5 users (show)

See Also:


Attachments
Testcase (439 bytes, image/svg+xml)
2018-01-22 22:23 PST, Simon Fraser (smfr)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-01-22 22:23:37 PST
Created attachment 332007 [details]
Testcase

When we reduce the filter resolution because of a large filter region, feFlood stops working. See test case.
Comment 1 Simon Fraser (smfr) 2018-01-31 09:19:14 PST
FilterEffect::apply() bails if ImageBuffer::sizeNeedsClamping() returns true, but we should have clamped already.
Comment 2 Simon Fraser (smfr) 2018-01-31 09:20:23 PST
Ah, rounding:

(lldb) fr va m_maxEffectRect
(WebCore::FloatRect) m_maxEffectRect = {
  m_location = (m_x = -20.4800014, m_y = -20.4800014)
  m_size = (m_width = 4096, m_height = 4096)
}
(lldb) fr va m_absolutePaintRect
(WebCore::IntRect) m_absolutePaintRect = {
  m_location = (m_x = -21, m_y = -21)
  m_size = (m_width = 4097, m_height = 4097)
}
Comment 3 Simon Fraser (smfr) 2018-01-31 10:01:20 PST
Need something like this:

diff --git a/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp b/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp
index 8ca9ada96d74b94ddf7b55a7f8fa070f617ae04c..dc21410a207996a7376bf1c32450d9c9e4007338 100644
--- a/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp
@@ -176,13 +176,16 @@ bool RenderSVGResourceFilter::applyResource(RenderElement& renderer, const Rende
     if (!lastEffect || lastEffect->totalNumberOfEffectInputs() > maxTotalOfEffectInputs)
         return false;
 
-    LOG_WITH_STREAM(Filters, stream << "RenderSVGResourceFilter::applyResource\n" << *filterData->builder->lastEffect());
-
     RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion(*lastEffect);
     FloatRect subRegion = lastEffect->maxEffectRect();
+
+    LOG_WITH_STREAM(Filters, stream << "RenderSVGResourceFilter::applyResource (scale " << scale << ", subregion " << subRegion << ")\n" << *filterData->builder->lastEffect());
+
     // At least one FilterEffect has a too big image size,
     // recalculate the effect sizes with new scale factors.
-    if (ImageBuffer::sizeNeedsClamping(subRegion.size(), scale)) {
+    auto boundingRect = enclosingIntRect(subRegion);
+    if (ImageBuffer::sizeNeedsClamping(boundingRect.size(), scale)) {
+        LOG_WITH_STREAM(Filters, stream << "subregion size " << subRegion.size() << " requires clamping, scaling by " << scale);
         filterData->filter->setFilterResolution(scale);
         RenderSVGResourceFilterPrimitive::determineFilterPrimitiveSubregion(*lastEffect);
     }
@@ -279,6 +282,9 @@ void RenderSVGResourceFilter::postApplyResource(RenderElement& renderer, Graphic
         // Always true if filterData is just built (filterData->state == FilterData::Built).
         if (!lastEffect->hasResult()) {
             filterData.state = FilterData::Applying;
+
+            LOG_WITH_STREAM(Filters, stream << "RenderSVGResourceFilter::postApplyResource " << *lastEffect);
+
             lastEffect->applyAll();
             lastEffect->correctFilterResultIfNeeded();
             lastEffect->transformResultColorSpace(ColorSpaceSRGB);

with a fudge factor for rounding.
Comment 4 Simon Fraser (smfr) 2018-02-01 21:35:46 PST
I think the clamping logic in ImageBuffer is basically wrong. It all works on FloatSizes, but ImageBuffers are allocated with integral numbers of pixels (despite taking FloatSize in the constructor). Also, clients can take a FloatRect and call enclosingIntRect() on it, potentially increasing the size by to 2px in each dimension.

So the ImageBuffer::sizeNeedsClamping() logic is suspect.