Diff
Modified: trunk/LayoutTests/ChangeLog (227373 => 227374)
--- trunk/LayoutTests/ChangeLog 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/LayoutTests/ChangeLog 2018-01-23 01:34:33 UTC (rev 227374)
@@ -1,3 +1,14 @@
+2018-01-22 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r226981): ASSERTION FAILED: startY >= 0 && endY <= height && startY < endY in WebCore::FEMorphology::platformApplyGeneric
+ https://bugs.webkit.org/show_bug.cgi?id=181836
+
+ Reviewed by Tim Horton.
+
+ * svg/filters/feLighting-parallel-jobs.svg: Added.
+ * svg/filters/feMorphology-invalid-radius.svg: Change the test to detect the bug on non-Retina too.
+ * svg/filters/feTurbulence-parallel-jobs-wide.svg: Added.
+
2018-01-22 Nikita Vasilyev <nvasil...@apple.com>
Web Inspector: Styles Redesign: data corruption when updating values quickly
Added: trunk/LayoutTests/svg/filters/feLighting-parallel-jobs-expected.txt (0 => 227374)
--- trunk/LayoutTests/svg/filters/feLighting-parallel-jobs-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/filters/feLighting-parallel-jobs-expected.txt 2018-01-23 01:34:33 UTC (rev 227374)
@@ -0,0 +1,2 @@
+Test passes if it does not assert in debug builds.
+
Added: trunk/LayoutTests/svg/filters/feLighting-parallel-jobs.svg (0 => 227374)
--- trunk/LayoutTests/svg/filters/feLighting-parallel-jobs.svg (rev 0)
+++ trunk/LayoutTests/svg/filters/feLighting-parallel-jobs.svg 2018-01-23 01:34:33 UTC (rev 227374)
@@ -0,0 +1,21 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+
+<defs>
+<filter id="light" primitiveUnits="userSpaceOnUse">
+<feSpecularLighting lighting-color="blue" surfaceScale="5" specularConstant="10" specularExponent="6">
+ <feDistantLight azimuth="0" elevation="30"/>
+</feSpecularLighting>
+</filter>
+</defs>
+
+<text y="20">Test passes if it does not assert in debug builds.</text>
+<rect x="0" y="30" height="10" width="10000" filter="url(#light)" fill="black" />
+
+<script>
+ <![CDATA[
+ if (window.testRunner)
+ testRunner.dumpAsText();
+]]>
+</script>
+
+</svg>
Modified: trunk/LayoutTests/svg/filters/feMorphology-invalid-radius.svg (227373 => 227374)
--- trunk/LayoutTests/svg/filters/feMorphology-invalid-radius.svg 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/LayoutTests/svg/filters/feMorphology-invalid-radius.svg 2018-01-23 01:34:33 UTC (rev 227374)
@@ -40,15 +40,16 @@
<g id="morphology">
<rect x="0" y="0" height="252" width="256" fill="green" />
- <rect x="0" y="0" height="28" width="256" fill="red" filter="url(#f1)" />
- <rect x="0" y="10" height="28" width="256" fill="red" filter="url(#f2)" />
- <rect x="0" y="20" height="28" width="256" fill="red" filter="url(#f3)" />
- <rect x="0" y="30" height="28" width="256" fill="red" filter="url(#f4)" />
- <rect x="0" y="40" height="28" width="256" fill="red" filter="url(#f5)" />
- <rect x="0" y="50" height="28" width="256" fill="red" filter="url(#f6)" />
- <rect x="0" y="60" height="28" width="256" fill="red" filter="url(#f7)" />
- <rect x="0" y="70" height="28" width="256" fill="red" filter="url(#f8)" />
- <rect x="0" y="80" height="28" width="256" fill="red" filter="url(#f9)" />
+ <rect x="0" y="0" height="4" width="512" fill="red" filter="url(#f1)" />
+ <rect x="0" y="10" height="46" width="512" fill="red" filter="url(#f2)" />
+ <rect x="0" y="20" height="46" width="512" fill="red" filter="url(#f3)" />
+ <rect x="0" y="30" height="46" width="512" fill="red" filter="url(#f4)" />
+ <rect x="0" y="40" height="46" width="512" fill="red" filter="url(#f5)" />
+ <rect x="0" y="50" height="46" width="512" fill="red" filter="url(#f6)" />
+ <rect x="0" y="60" height="46" width="512" fill="red" filter="url(#f7)" />
+ <rect x="0" y="70" height="46" width="512" fill="red" filter="url(#f8)" />
+ <rect x="0" y="80" height="46" width="512" fill="red" filter="url(#f9)" />
+ <rect x="0" y="90" height="46" width="512" fill="red" filter="url(#f4)" />
</g>
</svg>
<script>
Added: trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide-expected.txt (0 => 227374)
--- trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide-expected.txt 2018-01-23 01:34:33 UTC (rev 227374)
@@ -0,0 +1,2 @@
+Test passes if it does not assert in debug builds.
+
Added: trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide.svg (0 => 227374)
--- trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide.svg (rev 0)
+++ trunk/LayoutTests/svg/filters/feTurbulence-parallel-jobs-wide.svg 2018-01-23 01:34:33 UTC (rev 227374)
@@ -0,0 +1,17 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+ <defs>
+ <filter id="light">
+ <feTurbulence seed="10" type="turbulence" baseFrequency="0.01" numOctaves="1" />
+ </filter>
+ </defs>
+
+ <text y="20">Test passes if it does not assert in debug builds.</text>
+ <rect x="0" y="30" width="10000" height="10" filter="url(#light)"/>
+
+ <script>
+ <![CDATA[
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ ]]>
+ </script>
+</svg>
Modified: trunk/Source/WebCore/ChangeLog (227373 => 227374)
--- trunk/Source/WebCore/ChangeLog 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/Source/WebCore/ChangeLog 2018-01-23 01:34:33 UTC (rev 227374)
@@ -1,3 +1,35 @@
+2018-01-22 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r226981): ASSERTION FAILED: startY >= 0 && endY <= height && startY < endY in WebCore::FEMorphology::platformApplyGeneric
+ https://bugs.webkit.org/show_bug.cgi?id=181836
+
+ Reviewed by Tim Horton.
+
+ All the filters that use ParallelJobs<> has the same type of bug where very wide but not tall
+ filter regions could result in computing an optimalThreadNumber that was greater than the
+ number of rows to process, which resulted in jobs with zero rows to process.
+
+ Since we split the work by rows, cap the maximum number of threads to height/8 so that each job
+ has at least 8 rows of pixels to process. Add some assertions to detect jobs with zero rows.
+
+ FEMorphology was also using implicit float -> int conversion to detect integer overflow of radius,
+ so change that to use explicit clamping.
+
+ Tests: svg/filters/feLighting-parallel-jobs.svg
+ svg/filters/feTurbulence-parallel-jobs-wide.svg
+
+ * platform/graphics/filters/FELighting.cpp:
+ (WebCore::FELighting::platformApplyGenericPaint):
+ (WebCore::FELighting::platformApplyGeneric):
+ * platform/graphics/filters/FEMorphology.cpp:
+ (WebCore::FEMorphology::platformApplyGeneric):
+ (WebCore::FEMorphology::platformApply):
+ (WebCore::FEMorphology::platformApplyDegenerate):
+ (WebCore::FEMorphology::platformApplySoftware):
+ * platform/graphics/filters/FETurbulence.cpp:
+ (WebCore::FETurbulence::fillRegion const):
+ (WebCore::FETurbulence::platformApplySoftware):
+
2018-01-22 Eric Carlson <eric.carl...@apple.com>
Resign NowPlaying status when no media element is eligible
Modified: trunk/Source/WebCore/platform/graphics/filters/FELighting.cpp (227373 => 227374)
--- trunk/Source/WebCore/platform/graphics/filters/FELighting.cpp 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/Source/WebCore/platform/graphics/filters/FELighting.cpp 2018-01-23 01:34:33 UTC (rev 227374)
@@ -1,6 +1,7 @@
/*
* Copyright (C) 2010 University of Szeged
* Copyright (C) 2010 Zoltan Herczeg
+ * Copyright (C) 2018 Apple Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -307,6 +308,7 @@
{
// Make sure startY is > 0 since we read from the previous row in the loop.
ASSERT(startY);
+ ASSERT(endY > startY);
for (int y = startY; y < endY; ++y) {
int rowStartOffset = y * data.widthMultipliedByPixelSize;
@@ -341,7 +343,9 @@
void FELighting::platformApplyGeneric(const LightingData& data, const LightSource::PaintingData& paintingData)
{
- int optimalThreadNumber = ((data.widthDecreasedByOne - 1) * (data.heightDecreasedByOne - 1)) / s_minimalRectDimension;
+ unsigned rowsToProcess = data.heightDecreasedByOne - 1;
+ unsigned maxNumThreads = rowsToProcess / 8;
+ unsigned optimalThreadNumber = std::min<unsigned>(((data.widthDecreasedByOne - 1) * rowsToProcess) / s_minimalRectDimension, maxNumThreads);
if (optimalThreadNumber > 1) {
// Initialize parallel jobs
WTF::ParallelJobs<PlatformApplyGenericParameters> parallelJobs(&platformApplyGenericWorker, optimalThreadNumber);
@@ -351,8 +355,8 @@
if (job > 1) {
// Split the job into "yStep"-sized jobs but there a few jobs that need to be slightly larger since
// yStep * jobs < total size. These extras are handled by the remainder "jobsWithExtra".
- const int yStep = (data.heightDecreasedByOne - 1) / job;
- const int jobsWithExtra = (data.heightDecreasedByOne - 1) % job;
+ const int yStep = rowsToProcess / job;
+ const int jobsWithExtra = rowsToProcess % job;
int yStart = 1;
for (--job; job >= 0; --job) {
Modified: trunk/Source/WebCore/platform/graphics/filters/FEMorphology.cpp (227373 => 227374)
--- trunk/Source/WebCore/platform/graphics/filters/FEMorphology.cpp 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/Source/WebCore/platform/graphics/filters/FEMorphology.cpp 2018-01-23 01:34:33 UTC (rev 227374)
@@ -4,7 +4,7 @@
* Copyright (C) 2005 Eric Seidel <e...@webkit.org>
* Copyright (C) 2009 Dirk Schulze <k...@webkit.org>
* Copyright (C) Research In Motion Limited 2010. All rights reserved.
- * Copyright (C) Apple Inc. 2017. All rights reserved.
+ * Copyright (C) Apple Inc. 2017-2018 All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -139,6 +139,8 @@
void FEMorphology::platformApplyGeneric(const PaintingData& paintingData, int startY, int endY)
{
+ ASSERT(endY > startY);
+
const auto& srcPixelArray = *paintingData.srcPixelArray;
auto& dstPixelArray = *paintingData.dstPixelArray;
@@ -188,16 +190,17 @@
float kernelFactor = sqrt(paintingData.radiusX * paintingData.radiusY) * 0.65;
static const int minimalArea = (160 * 160); // Empirical data limit for parallel jobs
- int optimalThreadNumber = (paintingData.width * paintingData.height * kernelFactor) / minimalArea;
-
+
+ unsigned maxNumThreads = paintingData.height / 8;
+ unsigned optimalThreadNumber = std::min<unsigned>((paintingData.width * paintingData.height * kernelFactor) / minimalArea, maxNumThreads);
if (optimalThreadNumber > 1) {
- ParallelJobs<PlatformApplyParameters> parallelJobs(&WebCore::FEMorphology::platformApplyWorker, optimalThreadNumber);
- int numOfThreads = parallelJobs.numberOfJobs();
+ WTF::ParallelJobs<PlatformApplyParameters> parallelJobs(&WebCore::FEMorphology::platformApplyWorker, optimalThreadNumber);
+ auto numOfThreads = parallelJobs.numberOfJobs();
if (numOfThreads > 1) {
// Split the job into "jobSize"-sized jobs but there a few jobs that need to be slightly larger since
// jobSize * jobs < total size. These extras are handled by the remainder "jobsWithExtra".
- const int jobSize = paintingData.height / numOfThreads;
- const int jobsWithExtra = paintingData.height % numOfThreads;
+ int jobSize = paintingData.height / numOfThreads;
+ int jobsWithExtra = paintingData.height % numOfThreads;
int currentY = 0;
for (int job = numOfThreads - 1; job >= 0; --job) {
PlatformApplyParameters& param = parallelJobs.parameter(job);
@@ -224,8 +227,9 @@
return true;
}
- // Input radius is equal to zero or the scaled radius is less than one.
- if (!m_radiusX || !m_radiusY) {
+ // FIXME: this should allow erode/dilate on one axis. webkit.org/b/181903.
+ // Also if both x radiusX and radiusY are zero, the result should be transparent black.
+ if (!radiusX || !radiusY) {
FilterEffect* in = inputEffect(0);
in->copyPremultipliedResult(dstPixelArray, imageRect);
return true;
@@ -245,7 +249,9 @@
setIsAlphaImage(in->isAlphaImage());
IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
- if (platformApplyDegenerate(*dstPixelArray, effectDrawingRect, m_radiusX, m_radiusY))
+
+ IntSize radius = flooredIntSize(FloatSize(m_radiusX, m_radiusY));
+ if (platformApplyDegenerate(*dstPixelArray, effectDrawingRect, radius.width(), radius.height()))
return;
Filter& filter = this->filter();
@@ -253,7 +259,7 @@
if (!srcPixelArray)
return;
- IntSize radius = flooredIntSize(filter.scaledByFilterResolution({ m_radiusX, m_radiusY }));
+ radius = flooredIntSize(filter.scaledByFilterResolution({ m_radiusX, m_radiusY }));
int radiusX = std::min(effectDrawingRect.width() - 1, radius.width());
int radiusY = std::min(effectDrawingRect.height() - 1, radius.height());
Modified: trunk/Source/WebCore/platform/graphics/filters/FETurbulence.cpp (227373 => 227374)
--- trunk/Source/WebCore/platform/graphics/filters/FETurbulence.cpp 2018-01-23 01:29:39 UTC (rev 227373)
+++ trunk/Source/WebCore/platform/graphics/filters/FETurbulence.cpp 2018-01-23 01:34:33 UTC (rev 227374)
@@ -5,7 +5,7 @@
* Copyright (C) 2009 Dirk Schulze <k...@webkit.org>
* Copyright (C) 2010 Renata Hodovan <r...@inf.u-szeged.hu>
* Copyright (C) 2011 Gabor Loki <l...@webkit.org>
- * Copyright (C) 2017 Apple Inc.
+ * Copyright (C) 2017-2018 Apple Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -27,11 +27,10 @@
#include "FETurbulence.h"
#include "Filter.h"
-#include <wtf/text/TextStream.h>
-
#include <runtime/Uint8ClampedArray.h>
#include <wtf/MathExtras.h>
#include <wtf/ParallelJobs.h>
+#include <wtf/text/TextStream.h>
namespace WebCore {
@@ -368,6 +367,8 @@
void FETurbulence::fillRegion(Uint8ClampedArray& pixelArray, const PaintingData& paintingData, StitchData stitchData, int startY, int endY) const
{
+ ASSERT(endY > startY);
+
IntRect filterRegion = absolutePaintRect();
FloatPoint point(0, filterRegion.y() + startY);
int indexOfPixelChannel = startY * (filterRegion.width() << 2);
@@ -411,18 +412,19 @@
PaintingData paintingData(m_seed, tileSize, baseFrequencyX, baseFrequencyY);
initPaint(paintingData);
- int height = absolutePaintRect().height();
-
auto area = absolutePaintRect().area();
if (area.hasOverflowed())
return;
- unsigned optimalThreadNumber = area.unsafeGet() / s_minimalRectDimension;
+ int height = absolutePaintRect().height();
+
+ unsigned maxNumThreads = height / 8;
+ unsigned optimalThreadNumber = std::min<unsigned>(area.unsafeGet() / s_minimalRectDimension, maxNumThreads);
if (optimalThreadNumber > 1) {
WTF::ParallelJobs<FillRegionParameters> parallelJobs(&WebCore::FETurbulence::fillRegionWorker, optimalThreadNumber);
// Fill the parameter array
- unsigned numJobs = parallelJobs.numberOfJobs();
+ auto numJobs = parallelJobs.numberOfJobs();
if (numJobs > 1) {
// Split the job into "stepY"-sized jobs, distributing the extra rows into the first "jobsWithExtra" jobs.
unsigned stepY = height / numJobs;
