Bug 141653 - Huge blur request causes WebKit to treat blur as a no-op
Summary: Huge blur request causes WebKit to treat blur as a no-op
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-02-16 10:34 PST by Brent Fulgham
Modified: 2015-02-17 15:56 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2015-02-16 10:34:57 PST
The test case "css3/filters/huge-blur-value.html" is treated as a no-op in ToT WebKit. Instead of performing a blur operation, WebKit incorrectly determines that the blur region does not intersect with the dirty rect for the paint operation.

In the safari-600.5-branch branch, we correctly determine that the two regions do intersect, and attempt to perform he blur operation -- though we time out due to the massive size of the blur operation.

Therefore, we have two problems:

1. In ToT, we incorrectly determine that there is no work to be done. The test "css3/filters/huge-blur-value.html" does not notice this and treats it as a pass.
2. In the "safari-600.5-branch", we correctly determine that a blur operation must be done, but it takes so long to perform that the test times out.

We need to correct the layout arithmetic in ToT so that we recognize that work must be done. We must also determine an upper bound for our blur operations so that a malformed blur request does not result in a timeout/locked up system.
Comment 1 Radar WebKit Bug Importer 2015-02-16 10:36:03 PST
<rdar://problem/19847707>
Comment 2 Brent Fulgham 2015-02-16 10:37:40 PST
In both cases (ToT and safari-600.5-branch), the regions look like the following:

filterBoxRect: (512, 512) of size 3200x3200.
dirtyRect: (0, 0) of size 51200x38400

We move the dirtyRect around a bit based on the blurring dimensions (which are quite big) leaving us with:
rectForRepaint: (-1024868352, -1024868352) of size 2049787904x2049775104

So far, both branches look the same. But in ToT we do our layout math using subpixel logic. And this makes a big difference.

We check the intersection of the ‘rectForRepaint’ and the filterBoxRect. If there is no intersection, we do nothing.

void LayoutRect::intersect(const LayoutRect& other)
{
    LayoutPoint newLocation(std::max(x(), other.x()), std::max(y(), other.y()));
    LayoutPoint newMaxPoint(std::min(maxX(), other.maxX()), std::min(maxY(), other.maxY()));

    // Return a clean empty rectangle for non-intersecting cases.
    if (newLocation.x() >= newMaxPoint.x() || newLocation.y() >= newMaxPoint.y()) {
        newLocation = LayoutPoint(0, 0);
        newMaxPoint = LayoutPoint(0, 0);
    }

    m_location = newLocation;
    m_size = newMaxPoint - newLocation;
}

The above code is the same for both branches, but the LayoutPoint class is different. This gives us the following resulting points:

ToT:
newLocation	(2147483647, 2147483647)
newMaxPoint	(3776, 3712)

Since the location is way past the max point, we throw out everything and move on.

safari-600.5-branch:
newLocation	(512, 512)
newMaxPoint	(3712, 3712)

Uh, oh! Now we have work to do.

These values differ because in safari-600.5-branch, the x() return value is a signed integer value for the purpose of comparison. In ToT, it is unsigned so we get these huge values (in this crazy case) that result in no intersection.
Comment 3 Brent Fulgham 2015-02-16 10:56:42 PST
It looks like Firefox behaves like our WebKit nightlies, rendering "So blurry..." in clear text with no blur.

Chrome Canary 42.0.2306.0 seems to render no output, or perhaps blurs the "So blurry..." text to the point where it looks like whitespace.
Comment 4 zalan 2015-02-17 15:56:45 PST
This works for me. Both blur and intersect work fine.