Skip to content

GitLab

T
tangled
Commit 97fcb105 authored by Justin Novosad's avatar Justin Novosad Committed by Commit Bot
Browse files
Options

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

This reverts commit b096dcaa.

Reason for reland: This was not the right CL to revert (it was not even in the failure regression range).  The source of the regression is https://chromium-review.googlesource.com/c/chromium/src/+/1014140, which was just reverted so that this can reland.  The CL adds the test that was regressed by the other CL. Because these two CLs were in CQ at the same time, the problem only got caught on the main waterfall.

Original change's description:
> 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/1014321
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#551050}

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

Change-Id: I01758853007f982b81f99494be2be717c7ff6096
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/1014423Reviewed-by: default avatarJustin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551096}
parent a6ff8860
Hide whitespace changes
Inline Side-by-side
Showing with 162 additions and 11 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 97fcb105
......@@ -1235,6 +1235,10 @@ 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/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.
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 ]
......
third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow-expected.html 0 → 100644
View file @ 97fcb105
<!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>
third_party/WebKit/LayoutTests/fast/webgl/canvas-toDataURL-premul-overflow.html 0 → 100644
View file @ 97fcb105
<!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>
third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/toDataURL-color-managed-round-trip.html 0 → 100644
View file @ 97fcb105
<!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>
third_party/blink/renderer/platform/graphics/image_data_buffer.cc
View file @ 97fcb105
......@@ -53,18 +53,36 @@ ImageDataBuffer::ImageDataBuffer(scoped_refptr<StaticBitmapImage> image) {
retained_image_ = image->PaintImageForCurrentFrame().GetSkImage();
if (!retained_image_)
return;
if (retained_image_->isTextureBacked()) {
retained_image_ = retained_image_->makeNonTextureImage();
if (!retained_image_)
if (retained_image_->isTextureBacked() ||
retained_image_->isLazyGenerated() ||
retained_image_->alphaType() != kUnpremul_SkAlphaType) {
// 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;
} 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;
size_ = IntSize(image->width(), image->height());
}
......@@ -107,7 +125,14 @@ bool ImageDataBuffer::EncodeImage(const String& mime_type,
SkJpegEncoder::Options options;
options.fQuality = ImageEncoder::ComputeJpegQuality(quality);
options.fAlphaOption = SkJpegEncoder::AlphaOption::kBlendOnBlack;
options.fBlendBehavior = SkTransferFunctionBehavior::kIgnore;
// When the gamma is linear (which is always the case with currently
// 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) {
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