feFlood doesn't work when filter resolution is reduced

[Simon Fraser (smfr)]

Created attachment 332007 [details]
Testcase

When we reduce the filter resolution because of a large filter region, feFlood stops working. See test case.

[Simon Fraser (smfr)]

FilterEffect::apply() bails if ImageBuffer::sizeNeedsClamping() returns true, but we should have clamped already.

[Simon Fraser (smfr)]

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)
}

[Simon Fraser (smfr)]

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.

[Simon Fraser (smfr)]

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.

[Ahmad Saleem]

I am able to reproduce this bug in Safari 16 and STP 154 where it does not show filter (greyish) across viewport like other browsers (Chrome Canary 108 and Firefox Nightly 107). Just wanted to update latest results. Thanks!

Also is it something LBSE tackles?