Skip to content

Commit

Permalink
Make createImageBitmap() take EXIF orientation into account correctly
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=231063
<rdar://problem/83753956>

Reviewed by Myles Maxfield and Said Abou-Hallawa.

LayoutTests/imported/w3c:

* web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation-expected.txt: Added.
* web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation.html: Added.
* web-platform-tests/html/canvas/element/manual/imagebitmap/resources/squares.jpg: Added.

Source/WebCore:

Test: imported/w3c/web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation.html

This makes us treat {imageOrientation:"none"} as meaning "apply EXIF
orientation without any additional transformation", and
{imageOrientation:"flipY"} as meaning "apply EXIF orientation and then
apply an additional vertical flip". This behavior matches Firefox;
whatwg/html#7210 is open on clarifying this
behavior in the HTML spec.

* html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createPromise):
(WebCore::ImageBitmap::createFromBuffer):
(WebCore::imageOrientationForOrientation): Deleted.
* html/ImageBitmapOptions.h:
(WebCore::ImageBitmapOptions::resolvedImageOrientation const):
* html/ImageBitmapOptions.idl:
* platform/graphics/ImageOrientation.h:
(WebCore::ImageOrientation::withFlippedY const):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@284436 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
heycam@apple.com committed Oct 19, 2021
1 parent bad7d8c commit 0eeae73
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 19 deletions.
12 changes: 12 additions & 0 deletions LayoutTests/imported/w3c/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
2021-10-18 Cameron McCormack <heycam@apple.com>

Make createImageBitmap() take EXIF orientation into account correctly
https://bugs.webkit.org/show_bug.cgi?id=231063
<rdar://problem/83753956>

Reviewed by Myles Maxfield and Said Abou-Hallawa.

* web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation-expected.txt: Added.
* web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation.html: Added.
* web-platform-tests/html/canvas/element/manual/imagebitmap/resources/squares.jpg: Added.

2021-10-18 Chris Dumez <cdumez@apple.com>

Resync web-platform-tests/tools from upstream
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

PASS createImageBitmap with EXIF rotation, imageOrientation none, and no cropping
PASS createImageBitmap with EXIF rotation, imageOrientation flipY, and no cropping
PASS createImageBitmap with EXIF rotation, imageOrientation none, and cropping
PASS createImageBitmap with EXIF rotation, imageOrientation flipY, and cropping

Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
<!DOCTYPE html>
<title>Test that createImageBitmap honors EXIF orientation</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>canvas { outline: 1px solid black; margin-right: 1em; }</style>
<body>
<script>
function loadImage(src) {
return new Promise(function(resolve) {
const image = new Image();
image.addEventListener("load", () => resolve(image), { once: true });
image.src = src;
});
}

function checkColors(ctx, w, h, expectedColors) {
let data = ctx.getImageData(0, 0, w, h).data;
for (let [row, col, r, g, b, a] of expectedColors) {
let x = col * 80 + 40;
let y = row * 80 + 40;
let i = (x + y * w) * 4;

let expected = [r, g, b, a];
let actual = [data[i], data[i + 1], data[i + 2], data[i + 3]];

assert_array_approx_equals(actual, expected, 1, `Pixel value at (${x},${y}) ${expected} =~ ${actual}.`);
}
}

async_test(function(t) {
const canvas = document.createElement("canvas");
canvas.width = 320;
canvas.height = 160;
document.body.append(canvas);

const ctx = canvas.getContext("2d");
loadImage("resources/squares.jpg")
.then((image) => createImageBitmap(image))
.then(t.step_func_done(function(imageBitmap) {
ctx.drawImage(imageBitmap, 0, 0);
checkColors(ctx, canvas.width, canvas.height, [
// row, col, r, g, b, a
[0, 0, 255, 0, 0, 255],
[0, 1, 0, 255, 0, 255],
[0, 2, 0, 0, 255, 255],
[0, 3, 0, 0, 0, 255],
[1, 0, 255, 128, 128, 255],
[1, 1, 128, 255, 128, 255],
[1, 2, 128, 128, 255, 255],
[1, 3, 128, 128, 128, 255],
]);
}));
}, "createImageBitmap with EXIF rotation, imageOrientation none, and no cropping");

async_test(function(t) {
const canvas = document.createElement("canvas");
canvas.width = 320;
canvas.height = 160;
document.body.append(canvas);

const ctx = canvas.getContext("2d");
loadImage("resources/squares.jpg")
.then((image) => createImageBitmap(image, { imageOrientation: "flipY" }))
.then(t.step_func_done(function(imageBitmap) {
ctx.drawImage(imageBitmap, 0, 0);
checkColors(ctx, canvas.width, canvas.height, [
// row, col, r, g, b, a
[0, 0, 255, 128, 128, 255],
[0, 1, 128, 255, 128, 255],
[0, 2, 128, 128, 255, 255],
[0, 3, 128, 128, 128, 255],
[1, 0, 255, 0, 0, 255],
[1, 1, 0, 255, 0, 255],
[1, 2, 0, 0, 255, 255],
[1, 3, 0, 0, 0, 255],
]);
}));
}, "createImageBitmap with EXIF rotation, imageOrientation flipY, and no cropping");

async_test(function(t) {
const canvas = document.createElement("canvas");
canvas.width = 160;
canvas.height = 160;
document.body.append(canvas);

const ctx = canvas.getContext("2d");
loadImage("resources/squares.jpg")
.then(image => createImageBitmap(image, 80, 0, 160, 160))
.then(t.step_func_done(function(imageBitmap) {
ctx.drawImage(imageBitmap, 0, 0);
checkColors(ctx, canvas.width, canvas.height, [
// row, col, r, g, b, a
[0, 0, 0, 255, 0, 255],
[0, 1, 0, 0, 255, 255],
[1, 0, 128, 255, 128, 255],
[1, 1, 128, 128, 255, 255],
]);
}));
}, "createImageBitmap with EXIF rotation, imageOrientation none, and cropping");

async_test(function(t) {
const canvas = document.createElement("canvas");
canvas.width = 160;
canvas.height = 160;
document.body.append(canvas);

const ctx = canvas.getContext("2d");
loadImage("resources/squares.jpg")
.then(image => createImageBitmap(image, 80, 0, 160, 160, { imageOrientation: "flipY" }))
.then(t.step_func_done(function(imageBitmap) {
ctx.drawImage(imageBitmap, 0, 0);
checkColors(ctx, canvas.width, canvas.height, [
// row, col, r, g, b, a
[0, 0, 128, 255, 128, 255],
[0, 1, 128, 128, 255, 255],
[1, 0, 0, 255, 0, 255],
[1, 1, 0, 0, 255, 255],
]);
}));
}, "createImageBitmap with EXIF rotation, imageOrientation flipY, and cropping");
</script>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
27 changes: 27 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
2021-10-18 Cameron McCormack <heycam@apple.com>

Make createImageBitmap() take EXIF orientation into account correctly
https://bugs.webkit.org/show_bug.cgi?id=231063
<rdar://problem/83753956>

Reviewed by Myles Maxfield and Said Abou-Hallawa.

Test: imported/w3c/web-platform-tests/html/canvas/element/manual/imagebitmap/createImageBitmap-exif-orientation.html

This makes us treat {imageOrientation:"none"} as meaning "apply EXIF
orientation without any additional transformation", and
{imageOrientation:"flipY"} as meaning "apply EXIF orientation and then
apply an additional vertical flip". This behavior matches Firefox;
https://github.com/whatwg/html/issues/7210 is open on clarifying this
behavior in the HTML spec.

* html/ImageBitmap.cpp:
(WebCore::ImageBitmap::createPromise):
(WebCore::ImageBitmap::createFromBuffer):
(WebCore::imageOrientationForOrientation): Deleted.
* html/ImageBitmapOptions.h:
(WebCore::ImageBitmapOptions::resolvedImageOrientation const):
* html/ImageBitmapOptions.idl:
* platform/graphics/ImageOrientation.h:
(WebCore::ImageOrientation::withFlippedY const):

2021-10-18 Jean-Yves Avenard <jya@apple.com>

WebM with invalid size should fail to load with error
Expand Down
31 changes: 14 additions & 17 deletions Source/WebCore/html/ImageBitmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,6 @@ static InterpolationQuality interpolationQualityForResizeQuality(ImageBitmapOpti
return InterpolationQuality::Default;
}

static ImageOrientation imageOrientationForOrientation(ImageBitmapOptions::Orientation orientation)
{
if (orientation == ImageBitmapOptions::Orientation::FlipY)
return ImageOrientation(ImageOrientation::OriginBottomLeft);
return ImageOrientation();
}

static AlphaPremultiplication alphaPremultiplicationForPremultiplyAlpha(ImageBitmapOptions::PremultiplyAlpha premultiplyAlpha)
{
// The default is to premultiply - this is the least surprising behavior.
Expand Down Expand Up @@ -373,21 +366,25 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
return;
}

auto imageForRender = cachedImage->imageForRenderer(imageElement->renderer());
if (!imageForRender) {
auto imageForRenderer = cachedImage->imageForRenderer(imageElement->renderer());
if (!imageForRenderer) {
promise.reject(InvalidStateError, "Cannot create ImageBitmap from image that can't be rendered");
return;
}

auto outputSize = outputSizeForSourceRectangle(sourceRectangle.returnValue(), options);
auto bitmapData = createImageBuffer(scriptExecutionContext, outputSize, bufferRenderingMode, imageForRender->colorSpace());
auto bitmapData = createImageBuffer(scriptExecutionContext, outputSize, bufferRenderingMode, imageForRenderer->colorSpace());
if (!bitmapData) {
resolveWithBlankImageBuffer(scriptExecutionContext, !taintsOrigin(*cachedImage), WTFMove(promise));
return;
}

auto orientation = imageForRenderer->orientation();
if (orientation == ImageOrientation::FromImage)
orientation = ImageOrientation::None;

FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImage(*imageForRenderer, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(orientation) });

// 9. If the origin of image's image is not the same origin as the origin specified by the
// entry settings object, then set the origin-clean flag of the ImageBitmap object's
Expand Down Expand Up @@ -455,7 +452,7 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
}

FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(ImageOrientation::None) });

// 5. Set the origin-clean flag of the ImageBitmap object's bitmap to the same value as
// the origin-clean flag of the canvas element's bitmap.
Expand Down Expand Up @@ -525,7 +522,7 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
c.clip(FloatRect(FloatPoint(), outputSize));
auto scaleX = float(outputSize.width()) / float(sourceRectangle.width());
auto scaleY = float(outputSize.height()) / float(sourceRectangle.height());
if (options.imageOrientation == ImageBitmapOptions::Orientation::FlipY) {
if (options.orientation == ImageBitmapOptions::Orientation::FlipY) {
c.scale(FloatSize(scaleX, -scaleY));
c.translate(IntPoint(-sourceRectangle.location().x(), sourceRectangle.location().y() - outputSize.height()));
} else {
Expand Down Expand Up @@ -588,7 +585,7 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
auto imageForRender = existingImageBitmap->buffer()->copyImage();

FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImage(*imageForRender, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(ImageOrientation::None) });

// 5. Set the origin-clean flag of the ImageBitmap object's bitmap to the same
// value as the origin-clean flag of the bitmap of the image argument.
Expand Down Expand Up @@ -768,7 +765,7 @@ void ImageBitmap::createFromBuffer(ScriptExecutionContext& scriptExecutionContex
}

FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImage(image, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImage(image, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(ImageOrientation::None) });

OptionSet<SerializationState> serializationState = SerializationState::OriginClean;
if (alphaPremultiplicationForPremultiplyAlpha(options.premultiplyAlpha) == AlphaPremultiplication::Premultiplied)
Expand Down Expand Up @@ -815,7 +812,7 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
// If no cropping, resizing, flipping, etc. are needed, then simply use the
// resulting ImageBuffer directly.
auto alphaPremultiplication = alphaPremultiplicationForPremultiplyAlpha(options.premultiplyAlpha);
if (sourceRectangle.returnValue().location().isZero() && sourceRectangle.returnValue().size() == imageData->size() && sourceRectangle.returnValue().size() == outputSize && options.imageOrientation == ImageBitmapOptions::Orientation::None) {
if (sourceRectangle.returnValue().location().isZero() && sourceRectangle.returnValue().size() == imageData->size() && sourceRectangle.returnValue().size() == outputSize && options.orientation == ImageBitmapOptions::Orientation::None) {
bitmapData->putPixelBuffer(imageData->pixelBuffer(), sourceRectangle.releaseReturnValue(), { }, alphaPremultiplication);

auto imageBitmap = create(ImageBitmapBacking(WTFMove(bitmapData)));
Expand All @@ -833,7 +830,7 @@ void ImageBitmap::createPromise(ScriptExecutionContext& scriptExecutionContext,
}
tempBitmapData->putPixelBuffer(imageData->pixelBuffer(), IntRect(0, 0, imageData->width(), imageData->height()), { }, alphaPremultiplication);
FloatRect destRect(FloatPoint(), outputSize);
bitmapData->context().drawImageBuffer(*tempBitmapData, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), imageOrientationForOrientation(options.imageOrientation) });
bitmapData->context().drawImageBuffer(*tempBitmapData, destRect, sourceRectangle.releaseReturnValue(), { interpolationQualityForResizeQuality(options.resizeQuality), options.resolvedImageOrientation(ImageOrientation::None) });

// 6.4.1. Resolve p with ImageBitmap.
auto imageBitmap = create({ WTFMove(bitmapData) });
Expand Down
8 changes: 7 additions & 1 deletion Source/WebCore/html/ImageBitmapOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#pragma once

#include "ImageOrientation.h"
#include <optional>

namespace WebCore {
Expand All @@ -35,12 +36,17 @@ struct ImageBitmapOptions {
enum class ColorSpaceConversion { None, Default };
enum class ResizeQuality { Pixelated, Low, Medium, High };

Orientation imageOrientation { Orientation::None };
Orientation orientation { Orientation::None };
PremultiplyAlpha premultiplyAlpha { PremultiplyAlpha::Default };
ColorSpaceConversion colorSpaceConversion { ColorSpaceConversion::Default };
std::optional<unsigned> resizeWidth;
std::optional<unsigned> resizeHeight;
ResizeQuality resizeQuality { ResizeQuality::Low };

ImageOrientation resolvedImageOrientation(ImageOrientation imageOrientation) const
{
return orientation == Orientation::FlipY ? imageOrientation.withFlippedY() : imageOrientation;
}
};

}
2 changes: 1 addition & 1 deletion Source/WebCore/html/ImageBitmapOptions.idl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ enum ColorSpaceConversion { "none", "default" };
enum ResizeQuality { "pixelated", "low", "medium", "high" };

dictionary ImageBitmapOptions {
ImageOrientation imageOrientation = "none";
[ImplementedAs=orientation] ImageOrientation imageOrientation = "none";
PremultiplyAlpha premultiplyAlpha = "default";
ColorSpaceConversion colorSpaceConversion = "default";
[EnforceRange] unsigned long resizeWidth;
Expand Down
30 changes: 30 additions & 0 deletions Source/WebCore/platform/graphics/ImageOrientation.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,36 @@ struct ImageOrientation {
return AffineTransform();
}

ImageOrientation withFlippedY() const
{
ASSERT(isValidEXIFOrientation(m_orientation));

switch (m_orientation) {
case FromImage:
ASSERT_NOT_REACHED();
return None;
case OriginTopLeft:
return OriginBottomLeft;
case OriginTopRight:
return OriginBottomRight;
case OriginBottomRight:
return OriginTopRight;
case OriginBottomLeft:
return OriginTopLeft;
case OriginLeftTop:
return OriginLeftBottom;
case OriginRightTop:
return OriginRightBottom;
case OriginRightBottom:
return OriginRightTop;
case OriginLeftBottom:
return OriginLeftTop;
}

ASSERT_NOT_REACHED();
return None;
}

private:
static const Orientation EXIFFirst = OriginTopLeft;
static const Orientation EXIFLast = OriginLeftBottom;
Expand Down

0 comments on commit 0eeae73

Please sign in to comment.