Commit b096dcaa authored by Avi Drissman's avatar Avi Drissman Committed by Commit Bot

Revert "Fix alpha-unpremultiply overflow in WebGL+toDataURL"

This reverts commit dcffdac6.

Reason for revert:
Breaks fast/webgl/canvas-toDataURL-premul-overflow.html on
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty/43191
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/17758
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20(dbg)/11632
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/45887
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.11/31889
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12%20(retina)/328

Original change's description:
> Fix alpha-unpremultiply overflow in WebGL+toDataURL
> 
> This change fixes a regression that was introduced in
> https://chromium-review.googlesource.com/c/chromium/src/+/780279
> Fixing the issue by switching sensitive use cases back to using
> SkImage::readPixels for safely performing the unpremul conversion
> 
> BUG=826878
> 
> Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: Ie4bc9ced4d7175ab308529f7dd7119f93378e34c
> Reviewed-on: https://chromium-review.googlesource.com/1003320
> Commit-Queue: Justin Novosad <junov@chromium.org>
> Reviewed-by: Fernando Serboncini <fserb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#550979}

TBR=junov@chromium.org,fserb@chromium.org,xlai@chromium.org

Change-Id: Ib53e4ac4573894ff93424b50af6314a51fee217a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 826878
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/1014321Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551050}
parent ee2a9e65
...@@ -1235,10 +1235,6 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onprog ...@@ -1235,10 +1235,6 @@ crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-onprog
crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-progress-events.html [ Crash Timeout Pass ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/upload-progress-events.html [ Crash Timeout Pass ]
crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/upload-onprogress-event.html [ Pass Crash ] crbug.com/736308 virtual/outofblink-cors/http/tests/xmlhttprequest/workers/upload-onprogress-event.html [ Pass Crash ]
# This test failed to rebaseline correctly on mac with "webkit-patch rebaseline cl".
# We'll have to rebaseline from the waterfall bots after it lands.
crbug.com/826878 [ Mac ] virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html [ Failure ]
# Timeout tests in dictionary order. # Timeout tests in dictionary order.
crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-preflight-status.any.html [ Pass Timeout ] crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-preflight-status.any.html [ Pass Timeout ]
crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-redirect-preflight.any.html [ Pass Timeout ] crbug.com/736308 virtual/outofblink-cors/external/wpt/fetch/api/cors/cors-redirect-preflight.any.html [ Pass Timeout ]
......
<!DOCTYPE html>
<html>
<body>
<canvas id="draw" style="background:black; border:solid" width="10" height="10"></canvas>
<img id="feedback-png" style="background:black; border:solid">
<img id="feedback-jpeg" style="background:black; border:solid">
<img id="feedback-webp" style="background:black; border:solid">
<script>
var canvas = document.getElementById("draw");
var gl = canvas.getContext("webgl", {premultipliedAlpha: true});
gl.clearColor(0.5, 1, 0.3, 0.6);
gl.clear(gl.COLOR_BUFFER_BIT);
var hiddenCanvas = document.createElement('canvas');
hiddenCanvas.width = hiddenCanvas.height = 10;
var hiddengl = hiddenCanvas.getContext("webgl", {premultipliedAlpha: true});
hiddengl.clearColor(0.5, 0.6, 0.3, 0.6); // green clamped to alpha
hiddengl.clear(gl.COLOR_BUFFER_BIT);
// Ref test requires encode/decode round trip in order to repro compression
// artifacts exactly.
document.getElementById("feedback-png").src = hiddenCanvas.toDataURL("image/png");
document.getElementById("feedback-jpeg").src = hiddenCanvas.toDataURL("image/jpeg");
document.getElementById("feedback-webp").src = hiddenCanvas.toDataURL("image/webp");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<body>
<canvas id="draw" style="background:black; border:solid" width="10" height="10"></canvas>
<img id="feedback-png" style="background:black; border:solid">
<img id="feedback-jpeg" style="background:black; border:solid">
<img id="feedback-webp" style="background:black; border:solid">
<script>
var canvas = document.getElementById("draw");
var gl = canvas.getContext("webgl", {premultipliedAlpha: true});
// green component larger than alpha: will cause an overflow when unmultiplied
gl.clearColor(0.5, 1, 0.3, 0.6);
gl.clear(gl.COLOR_BUFFER_BIT);
document.getElementById("feedback-png").src = canvas.toDataURL("image/png");
document.getElementById("feedback-jpeg").src = canvas.toDataURL("image/jpeg");
document.getElementById("feedback-webp").src = canvas.toDataURL("image/webp");
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<style>
div {
margin:0px;
padding:0px;
}
canvas {
background:black;
margin:1px;
}
img {
background:black;
margin:1px;
}
</style>
<body>
<div id="results"></div>
<script>
if (window.testRunner) {
testRunner.dumpAsTextWithPixelResults();
testRunner.waitUntilDone();
}
// This test should produce a 5x4 grid of identical squares
// TODO(crbug.com/831254)
let canvasAttributes = [
{},
{colorSpace: 'srgb', pixelFormat: '8-8-8-8'},
{colorSpace: 'srgb', pixelFormat: 'float16'},
{colorSpace: 'rec2020', pixelFormat: 'float16'},
{colorSpace: 'p3', pixelFormat: 'float16'}
];
let resultParent = document.getElementById("results");
for (let i = 0; i < canvasAttributes.length; i++) {
let result = document.createElement("div");
resultParent.appendChild(result);
let canvas = document.createElement("canvas");
result.appendChild(canvas);
let pngImg = document.createElement("img");
result.appendChild(pngImg);
let jpegImg = document.createElement("img");
result.appendChild(jpegImg);
let webpImg = document.createElement("img");
result.appendChild(webpImg);
canvas.width = canvas.height = 10;
let ctx = canvas.getContext("2d", canvasAttributes[i]);
ctx.fillStyle = "rgb(255, 100, 10)";
ctx.fillRect(0, 0, 5, 10);
ctx.fillStyle = "rgba(255, 100, 10, 0.5)";
ctx.fillRect(5, 0, 5, 10);
pngImg.src = canvas.toDataURL("image/png");
jpegImg.src = canvas.toDataURL("image/jpeg");
webpImg.src = canvas.toDataURL("image/webp");
pngImg.onload = loadedOneImage;
jpegImg.onload = loadedOneImage;
webpImg.onload = loadedOneImage;
}
var imagesLoaded = 0;
function loadedOneImage() {
imagesLoaded = imagesLoaded + 1;
if (imagesLoaded >= 3 * canvasAttributes.length) {
if (window.testRunner)
testRunner.notifyDone();
}
}
</script>
</body>
</html>
...@@ -53,36 +53,18 @@ ImageDataBuffer::ImageDataBuffer(scoped_refptr<StaticBitmapImage> image) { ...@@ -53,36 +53,18 @@ ImageDataBuffer::ImageDataBuffer(scoped_refptr<StaticBitmapImage> image) {
retained_image_ = image->PaintImageForCurrentFrame().GetSkImage(); retained_image_ = image->PaintImageForCurrentFrame().GetSkImage();
if (!retained_image_) if (!retained_image_)
return; return;
if (retained_image_->isTextureBacked() || if (retained_image_->isTextureBacked()) {
retained_image_->isLazyGenerated() || retained_image_ = retained_image_->makeNonTextureImage();
retained_image_->alphaType() != kUnpremul_SkAlphaType) { if (!retained_image_)
// Unpremul is handled upfront, using readPixels, which will correctly clamp
// premul color values that would otherwise cause overflows in the skia
// encoder unpremul logic.
SkColorType colorType = retained_image_->colorType();
if (colorType == kRGBA_8888_SkColorType ||
colorType == kBGRA_8888_SkColorType)
colorType = kN32_SkColorType; // Work around for bug with JPEG encoder
const SkImageInfo info =
SkImageInfo::Make(retained_image_->width(), retained_image_->height(),
retained_image_->colorType(), kUnpremul_SkAlphaType,
retained_image_->refColorSpace());
const size_t rowBytes = info.minRowBytes();
size_t size = info.computeByteSize(rowBytes);
if (SkImageInfo::ByteSizeOverflowed(size))
return;
sk_sp<SkData> data = SkData::MakeUninitialized(size);
pixmap_ = {info, data->writable_data(), info.minRowBytes()};
if (!retained_image_->readPixels(pixmap_, 0, 0)) {
pixmap_.reset();
return;
}
retained_image_ = SkImage::MakeRasterData(info, std::move(data), rowBytes);
} else {
if (!retained_image_->peekPixels(&pixmap_))
return; return;
} else if (retained_image_->isLazyGenerated()) {
// Call readPixels() to trigger decoding.
SkImageInfo info = SkImageInfo::MakeN32(1, 1, retained_image_->alphaType());
std::unique_ptr<uint8_t[]> pixel(new uint8_t[info.bytesPerPixel()]());
retained_image_->readPixels(info, pixel.get(), info.minRowBytes(), 1, 1);
} }
if (!retained_image_->peekPixels(&pixmap_))
return;
is_valid_ = true; is_valid_ = true;
size_ = IntSize(image->width(), image->height()); size_ = IntSize(image->width(), image->height());
} }
...@@ -125,14 +107,7 @@ bool ImageDataBuffer::EncodeImage(const String& mime_type, ...@@ -125,14 +107,7 @@ bool ImageDataBuffer::EncodeImage(const String& mime_type,
SkJpegEncoder::Options options; SkJpegEncoder::Options options;
options.fQuality = ImageEncoder::ComputeJpegQuality(quality); options.fQuality = ImageEncoder::ComputeJpegQuality(quality);
options.fAlphaOption = SkJpegEncoder::AlphaOption::kBlendOnBlack; options.fAlphaOption = SkJpegEncoder::AlphaOption::kBlendOnBlack;
// When the gamma is linear (which is always the case with currently options.fBlendBehavior = SkTransferFunctionBehavior::kIgnore;
// supported color spaces in F16 format), it does not matter whether we use
// kRespect or kIgnore, but the JPEG encoder does not support kIgnore with
// F16 for some reason, so we switch to kRespect in that case, with no
// consequence on the encoded output.
options.fBlendBehavior = pixmap_.colorType() == kRGBA_F16_SkColorType
? SkTransferFunctionBehavior::kRespect
: SkTransferFunctionBehavior::kIgnore;
if (options.fQuality == 100) { if (options.fQuality == 100) {
options.fDownsample = SkJpegEncoder::Downsample::k444; options.fDownsample = SkJpegEncoder::Downsample::k444;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment