Diff
Modified: branches/safari-536.28-branch/LayoutTests/ChangeLog (134269 => 134270)
--- branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/LayoutTests/ChangeLog 2012-11-12 19:36:37 UTC (rev 134270)
@@ -1,3 +1,20 @@
+2012-11-12 Lucas Forschler <[email protected]>
+
+ Merge r129796
+
+ 2012-09-27 Philip Rogers <[email protected]>
+
+ Rewrite multithreaded filter job dispatching
+ https://bugs.webkit.org/show_bug.cgi?id=97500
+
+ Reviewed by Dean Jackson.
+
+ Add test that shows the morphology filter no longer crashes.
+ Note: this is only reliably reproducable when run with libgmalloc.
+
+ * svg/filters/feMorphology-crash-expected.txt: Added.
+ * svg/filters/feMorphology-crash.html: Added.
+
2012-11-09 Lucas Forschler <[email protected]>
Merge r132427
@@ -11324,3 +11341,4 @@
.
.
.
+.
Copied: branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash-expected.txt (from rev 129796, trunk/LayoutTests/svg/filters/feMorphology-crash-expected.txt) (0 => 134270)
--- branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash-expected.txt (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash-expected.txt 2012-11-12 19:36:37 UTC (rev 134270)
@@ -0,0 +1 @@
+PASS - This test passes if it did not crash.
Copied: branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash.html (from rev 129796, trunk/LayoutTests/svg/filters/feMorphology-crash.html) (0 => 134270)
--- branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash.html (rev 0)
+++ branches/safari-536.28-branch/LayoutTests/svg/filters/feMorphology-crash.html 2012-11-12 19:36:37 UTC (rev 134270)
@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html>
+<!-- Test for WK97500 - this test passes if it does not crash. -->
+<script>
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ setTimeout(function() {
+ document.write("PASS - This test passes if it did not crash.");
+ testRunner.dumpAsText();
+ testRunner.notifyDone();
+ }, 1);
+ }
+</script>
+<svg xmlns:xlink="http://www.w3.org/1999/xlink" xmlns="http://www.w3.org/2000/svg" width="300" height="300">
+ <defs>
+ <filter id="erode1" filterUnits="objectBoundingBox" x="0" y="0" width="1000" height="1000">
+ <feMorphology operator="erode" radius="1" width="2000" height="100000"/>
+ </filter>
+ <g id="morphologySource">
+ <rect x="0" y="0" width="2000" height="100000"/>
+ </g>
+ </defs>
+ <use xlink:href="" x="350" y="100" filter="url(#erode1)"/>
+</svg>
+</html>
Modified: branches/safari-536.28-branch/Source/WebCore/ChangeLog (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/ChangeLog 2012-11-12 19:36:37 UTC (rev 134270)
@@ -1,3 +1,51 @@
+2012-11-12 Lucas Forschler <[email protected]>
+
+ Merge r129796
+
+ 2012-09-27 Philip Rogers <[email protected]>
+
+ Rewrite multithreaded filter job dispatching
+ https://bugs.webkit.org/show_bug.cgi?id=97500
+
+ Reviewed by Dean Jackson.
+
+ This patch solves the problem of splitting up images into subregions for multithreaded
+ filters. This fixes the way we partition the image array into equal-sized chunks.
+ If we have an array of length N and want to split it into K chunks, we calculate:
+ int jobSize = N / K; // integer division, so this is floored
+ int jobSizeExtra = N % K; // modulus produces the remainder
+ We then split the array into jobSizeExtra number of jobs with size jobSize + 1
+ and (K - jobSizeExtra) number of jobs with size jobSize. This pattern
+ is used in each of the 5 filters in this patch.
+
+ This patch primarily fixes an error in FEMorphology::platformApply where
+ the image array was partitioned into (1 + (N / K)) pieces with the last job
+ taking the remainder. Unfortunately, this can cause overruns in the 2nd-to-last job.
+ Consider N = 2373 and K = 64 jobs. Job 0 would take indices 0...38, job 1 would take
+ 38...76, etc. Unfortunately the 62nd job takes 2356...2394 which overruns.
+
+ To prevent similar issues elsewhere this patch updates all of the filters
+ to use the same pattern as FEMorphology.
+
+ Test: svg/filters/feMorphology-crash.html
+
+ * platform/graphics/filters/FEConvolveMatrix.cpp:
+ (WebCore::FEConvolveMatrix::platformApplySoftware):
+ * platform/graphics/filters/FEGaussianBlur.cpp:
+ (WebCore::FEGaussianBlur::platformApply):
+ * platform/graphics/filters/FELighting.cpp:
+ (WebCore::FELighting::platformApplyGeneric):
+ * platform/graphics/filters/FEMorphology.cpp:
+ (WebCore::FEMorphology::platformApply):
+
+ Some special care is taken for Gaussian Blur because there is an
+ extraHeight parameter for sampling outside the image's dimensions.
+ This means we use the same partitioning algorithm but add
+ extraHeight padding on the lower and upper bounds.
+
+ * platform/graphics/filters/FETurbulence.cpp:
+ (WebCore::FETurbulence::platformApplySoftware):
+
2012-11-09 Lucas Forschler <[email protected]>
Merge r132427
@@ -207268,3 +207316,4 @@
.
.
.
+.
Modified: branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEConvolveMatrix.cpp (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEConvolveMatrix.cpp 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEConvolveMatrix.cpp 2012-11-12 19:36:37 UTC (rev 134270)
@@ -444,9 +444,13 @@
if (optimalThreadNumber > 1) {
WTF::ParallelJobs<InteriorPixelParameters> parallelJobs(&WebCore::FEConvolveMatrix::setInteriorPixelsWorker, optimalThreadNumber);
const int numOfThreads = parallelJobs.numberOfJobs();
+
+ // Split the job into "heightPerThread" jobs but there a few jobs that need to be slightly larger since
+ // heightPerThread * jobs < total size. These extras are handled by the remainder "jobsWithExtra".
const int heightPerThread = clipBottom / numOfThreads;
+ const int jobsWithExtra = clipBottom % numOfThreads;
+
int startY = 0;
-
for (int job = 0; job < numOfThreads; ++job) {
InteriorPixelParameters& param = parallelJobs.parameter(job);
param.filter = this;
@@ -454,11 +458,8 @@
param.clipRight = clipRight;
param.clipBottom = clipBottom;
param.yStart = startY;
- if (job < numOfThreads - 1) {
- startY += heightPerThread;
- param.yEnd = startY - 1;
- } else
- param.yEnd = clipBottom;
+ startY += job < jobsWithExtra ? heightPerThread + 1 : heightPerThread;
+ param.yEnd = startY;
}
parallelJobs.execute();
Modified: branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEGaussianBlur.cpp (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEGaussianBlur.cpp 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEGaussianBlur.cpp 2012-11-12 19:36:37 UTC (rev 134270)
@@ -162,29 +162,25 @@
int jobs = parallelJobs.numberOfJobs();
if (jobs > 1) {
- int blockHeight = paintSize.height() / jobs;
- --jobs;
- for (int job = jobs; job >= 0; --job) {
+ // Split the job into "blockHeight"-sized jobs but there a few jobs that need to be slightly larger since
+ // blockHeight * jobs < total size. These extras are handled by the remainder "jobsWithExtra".
+ const int blockHeight = paintSize.height() / jobs;
+ const int jobsWithExtra = paintSize.height() % jobs;
+
+ int currentY = 0;
+ for (int job = 0; job < jobs; job++) {
PlatformApplyParameters& params = parallelJobs.parameter(job);
params.filter = this;
- int startY;
- int endY;
+ int startY = !job ? 0 : currentY - extraHeight;
+ currentY += job < jobsWithExtra ? blockHeight + 1 : blockHeight;
+ int endY = job == jobs - 1 ? currentY : currentY + extraHeight;
+
+ int blockSize = (endY - startY) * scanline;
if (!job) {
- startY = 0;
- endY = blockHeight + extraHeight;
params.srcPixelArray = srcPixelArray;
params.dstPixelArray = tmpPixelArray;
} else {
- if (job == jobs) {
- startY = job * blockHeight - extraHeight;
- endY = paintSize.height();
- } else {
- startY = job * blockHeight - extraHeight;
- endY = (job + 1) * blockHeight + extraHeight;
- }
-
- int blockSize = (endY - startY) * scanline;
params.srcPixelArray = Uint8ClampedArray::createUninitialized(blockSize);
params.dstPixelArray = Uint8ClampedArray::createUninitialized(blockSize);
memcpy(params.srcPixelArray->data(), srcPixelArray->data() + startY * scanline, blockSize);
@@ -199,20 +195,19 @@
parallelJobs.execute();
// Copy together the parts of the image.
- for (int job = jobs; job >= 1; --job) {
+ currentY = 0;
+ for (int job = 1; job < jobs; job++) {
PlatformApplyParameters& params = parallelJobs.parameter(job);
int sourceOffset;
int destinationOffset;
int size;
- if (job == jobs) {
- sourceOffset = extraHeight * scanline;
- destinationOffset = job * blockHeight * scanline;
- size = (paintSize.height() - job * blockHeight) * scanline;
- } else {
- sourceOffset = extraHeight * scanline;
- destinationOffset = job * blockHeight * scanline;
- size = blockHeight * scanline;
- }
+ int adjustedBlockHeight = job < jobsWithExtra ? blockHeight + 1 : blockHeight;
+
+ currentY += adjustedBlockHeight;
+ sourceOffset = extraHeight * scanline;
+ destinationOffset = currentY * scanline;
+ size = adjustedBlockHeight * scanline;
+
memcpy(srcPixelArray->data() + destinationOffset, params.srcPixelArray->data() + sourceOffset, size);
}
return;
Modified: branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FELighting.cpp (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FELighting.cpp 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FELighting.cpp 2012-11-12 19:36:37 UTC (rev 134270)
@@ -257,19 +257,20 @@
// Fill the parameter array
int job = parallelJobs.numberOfJobs();
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;
+
int yStart = 1;
- int yStep = (data.heightDecreasedByOne - 1) / job;
for (--job; job >= 0; --job) {
PlatformApplyGenericParameters& params = parallelJobs.parameter(job);
params.filter = this;
params.data = ""
params.paintingData = paintingData;
params.yStart = yStart;
- if (job > 0) {
- params.yEnd = yStart + yStep;
- yStart += yStep;
- } else
- params.yEnd = data.heightDecreasedByOne;
+ yStart += job < jobsWithExtra ? yStep + 1 : yStep;
+ params.yEnd = yStart;
}
parallelJobs.execute();
return;
Modified: branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEMorphology.cpp (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEMorphology.cpp 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FEMorphology.cpp 2012-11-12 19:36:37 UTC (rev 134270)
@@ -173,14 +173,17 @@
ParallelJobs<PlatformApplyParameters> parallelJobs(&WebCore::FEMorphology::platformApplyWorker, optimalThreadNumber);
int numOfThreads = parallelJobs.numberOfJobs();
if (numOfThreads > 1) {
- const int deltaY = 1 + paintingData->height / numOfThreads;
+ // 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 currentY = 0;
for (int job = numOfThreads - 1; job >= 0; --job) {
PlatformApplyParameters& param = parallelJobs.parameter(job);
param.filter = this;
param.startY = currentY;
- currentY += deltaY;
- param.endY = job ? currentY : paintingData->height;
+ currentY += job < jobsWithExtra ? jobSize + 1 : jobSize;
+ param.endY = currentY;
param.paintingData = paintingData;
}
parallelJobs.execute();
Modified: branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FETurbulence.cpp (134269 => 134270)
--- branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FETurbulence.cpp 2012-11-12 19:31:41 UTC (rev 134269)
+++ branches/safari-536.28-branch/Source/WebCore/platform/graphics/filters/FETurbulence.cpp 2012-11-12 19:36:37 UTC (rev 134270)
@@ -376,19 +376,20 @@
// Fill the parameter array
int i = parallelJobs.numberOfJobs();
if (i > 1) {
+ // Split the job into "stepY"-sized jobs but there a few jobs that need to be slightly larger since
+ // stepY * jobs < total size. These extras are handled by the remainder "jobsWithExtra".
+ const int stepY = absolutePaintRect().height() / i;
+ const int jobsWithExtra = absolutePaintRect().height() % i;
+
int startY = 0;
- int stepY = absolutePaintRect().height() / i;
for (; i > 0; --i) {
FillRegionParameters& params = parallelJobs.parameter(i-1);
params.filter = this;
params.pixelArray = pixelArray;
params.paintingData = &paintingData;
params.startY = startY;
- if (i != 1) {
- params.endY = startY + stepY;
- startY = startY + stepY;
- } else
- params.endY = absolutePaintRect().height();
+ startY += i < jobsWithExtra ? stepY + 1 : stepY;
+ params.endY = startY;
}
// Execute parallel jobs
