SkAnimatedImage: Always respect exif orientation

No known clients (CanvasKit, Android) want to ignore the exif
orientation in an animation.

A follow-on CL will deprecate SkAndroidCodec::ExifOrientation, leaving
it up to the client (e.g. SkAnimatedImage, hwui/ImageDecoder) to
handle the orientation. Add SkEncodedOriginSwapsWidthHeight to assist
clients to do so.

Update stoplight_animated_image GM. It previously showed using
SkAnimatedImage without respecting the orientation, which is no longer
supported. The new version replaces the left half of the image with the
right.

Remove assert that is no longer true. Originally, an SkAnimatedImage was
"simple" if it did not have a crop or postProcessor. This is no longer
true if has an exif orientation. Add a test that calls the simple
constructor and verifies it does not crash.

Change-Id: I421fd02700f220fb90458cd03c4431dee7daf399
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344762
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This commit is contained in:
Leon Scroggins III 2020-12-15 15:52:39 -05:00 committed by Skia Commit-Bot
parent 6aea078802
commit e42716032b
14 changed files with 88 additions and 87 deletions

View File

@ -6,6 +6,12 @@ This file includes a list of high level updates for each milestone release.
Milestone 89
------------
* SkAnimatedImage: Always respect exif orientation
Replace SkPixmapPriv::ShouldSwapWidthHeight with
SkEncodedOriginSwapsWidthHeight.
https://review.skia.org/344762
* Add kDirectionalLight_ShadowFlag support. If enabled, light position represents
a vector pointing towards the light, and light radius is blur radius at elevation 1.
https://review.skia.org/321792

View File

@ -57,9 +57,7 @@ class AnimatedImageGM : public skiagm::GM {
* 1.25f // will be scaled up
+ 2; // padding
auto origin = codec->getOrigin();
bool respectOrigin = origin != kDefault_SkEncodedOrigin;
fSize = { fTranslate * kMaxFrames * (respectOrigin ? 2 : 1)
fSize = { fTranslate * kMaxFrames
* 2 // crop and no-crop
* 2, // post-process and no post-process
fTranslate * 4 // 4 scales
@ -100,67 +98,55 @@ public:
};
for (float scale : { 1.25f, 1.0f, .75f, .5f }) {
canvas->save();
for (auto behavior : { SkAndroidCodec::ExifOrientationBehavior::kIgnore,
SkAndroidCodec::ExifOrientationBehavior::kRespect }) {
// Only use kRespect if the origin is not the default.
bool needRespect = true;
for (bool doCrop : { false, true }) {
for (bool doPostProcess : { false, true }) {
auto codec = SkCodec::MakeFromData(fData);
const auto origin = codec->getOrigin();
if (origin == kDefault_SkEncodedOrigin) {
needRespect = false;
}
auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec),
behavior);
auto info = androidCodec->getInfo();
const SkISize unscaledSize = info.dimensions();
for (bool doCrop : { false, true }) {
for (bool doPostProcess : { false, true }) {
auto codec = SkCodec::MakeFromData(fData);
const auto origin = codec->getOrigin();
auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec));
auto info = androidCodec->getInfo();
const auto unscaledSize = SkEncodedOriginSwapsWidthHeight(origin)
? SkISize{ info.height(), info.width() } : info.dimensions();
SkISize scaledSize = { SkScalarFloorToInt(info.width() * scale) ,
SkScalarFloorToInt(info.height() * scale) };
info = info.makeDimensions(scaledSize);
SkISize scaledSize = { SkScalarFloorToInt(unscaledSize.width() * scale) ,
SkScalarFloorToInt(unscaledSize.height() * scale) };
info = info.makeDimensions(scaledSize);
auto cropRect = SkIRect::MakeSize(scaledSize);
if (doCrop) {
auto matrix = SkMatrix::MakeRectToRect(SkRect::Make(unscaledSize),
SkRect::Make(scaledSize), SkMatrix::kFill_ScaleToFit);
if (behavior == SkAndroidCodec::ExifOrientationBehavior::kRespect
&& needRespect) {
matrix.preConcat(SkEncodedOriginToMatrix(origin,
unscaledSize.width(), unscaledSize.height()));
auto cropRect = SkIRect::MakeSize(scaledSize);
if (doCrop) {
auto matrix = SkMatrix::MakeRectToRect(SkRect::Make(unscaledSize),
SkRect::Make(scaledSize), SkMatrix::kFill_ScaleToFit);
matrix.preConcat(SkEncodedOriginToMatrix(origin,
unscaledSize.width(), unscaledSize.height()));
SkRect cropRectFloat = SkRect::Make(fCropRect);
matrix.mapRect(&cropRectFloat);
cropRectFloat.roundOut(&cropRect);
}
sk_sp<SkPicture> postProcessor = doPostProcess
? post_processor(SkRect::Make(cropRect.size())) : nullptr;
auto animatedImage = SkAnimatedImage::Make(std::move(androidCodec),
info, cropRect, std::move(postProcessor));
animatedImage->setRepetitionCount(0);
for (int frame = 0; frame < kMaxFrames; frame++) {
{
SkAutoCanvasRestore acr(canvas, doCrop);
if (doCrop) {
canvas->translate(cropRect.left(), cropRect.top());
}
SkRect cropRectFloat = SkRect::Make(fCropRect);
matrix.mapRect(&cropRectFloat);
cropRectFloat.roundOut(&cropRect);
drawProc(animatedImage);
}
sk_sp<SkPicture> postProcessor = doPostProcess
? post_processor(SkRect::Make(cropRect.size())) : nullptr;
auto animatedImage = SkAnimatedImage::Make(std::move(androidCodec),
info, cropRect, std::move(postProcessor));
animatedImage->setRepetitionCount(0);
for (int frame = 0; frame < kMaxFrames; frame++) {
{
SkAutoCanvasRestore acr(canvas, doCrop);
if (doCrop) {
canvas->translate(cropRect.left(), cropRect.top());
}
drawProc(animatedImage);
}
canvas->translate(fTranslate, 0);
const auto duration = animatedImage->currentFrameDuration();
if (duration == SkAnimatedImage::kFinished) {
break;
}
for (int i = 0; i < fStep; i++) {
animatedImage->decodeNextFrame();
}
canvas->translate(fTranslate, 0);
const auto duration = animatedImage->currentFrameDuration();
if (duration == SkAnimatedImage::kFinished) {
break;
}
for (int i = 0; i < fStep; i++) {
animatedImage->decodeNextFrame();
}
}
}
if (!needRespect) break;
}
canvas->restore();
canvas->translate(0, fTranslate);

View File

@ -270,7 +270,5 @@ private:
const SkImageInfo fInfo;
const ExifOrientationBehavior fOrientationBehavior;
std::unique_ptr<SkCodec> fCodec;
friend class SkAnimatedImage;
};
#endif // SkAndroidCodec_DEFINED

View File

@ -43,5 +43,12 @@ static inline SkMatrix SkEncodedOriginToMatrix(SkEncodedOrigin origin, int w, in
SK_ABORT("Unexpected origin");
}
/**
* Return true if the encoded origin includes a 90 degree rotation, in which case the width
* and height of the source data are swapped relative to a correctly oriented destination.
*/
static inline bool SkEncodedOriginSwapsWidthHeight(SkEncodedOrigin origin) {
return origin >= kLeftTop_SkEncodedOrigin;
}
#endif // SkEncodedOrigin_DEFINED

View File

@ -753,10 +753,7 @@ EMSCRIPTEN_BINDINGS(Skia) {
size_t length)->sk_sp<SkAnimatedImage> {
uint8_t* imgData = reinterpret_cast<uint8_t*>(iptr);
auto bytes = SkData::MakeFromMalloc(imgData, length);
auto stream = SkMemoryStream::Make(std::move(bytes));
auto codec = SkCodec::MakeFromStream(std::move(stream), nullptr, nullptr);
auto aCodec = SkAndroidCodec::MakeFromCodec(std::move(codec),
SkAndroidCodec::ExifOrientationBehavior::kRespect);
auto aCodec = SkAndroidCodec::MakeFromData(std::move(bytes));
if (nullptr == aCodec) {
return nullptr;
}

View File

@ -46,10 +46,7 @@ sk_sp<SkAnimatedImage> SkAnimatedImage::Make(std::unique_ptr<SkAndroidCodec> cod
const auto& decodeInfo = codec->getInfo();
const auto cropRect = SkIRect::MakeSize(decodeInfo.dimensions());
auto image = Make(std::move(codec), decodeInfo, cropRect, nullptr);
SkASSERT(!image || image->simple());
return image;
return Make(std::move(codec), decodeInfo, cropRect, nullptr);
}
SkAnimatedImage::SkAnimatedImage(std::unique_ptr<SkAndroidCodec> codec,
@ -72,14 +69,11 @@ SkAnimatedImage::SkAnimatedImage(std::unique_ptr<SkAndroidCodec> codec,
// and applied in the following order:
// [crop] X [origin] X [scale]
const auto origin = fCodec->codec()->getOrigin();
if (fCodec->fOrientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kRespect
&& origin != SkEncodedOrigin::kDefault_SkEncodedOrigin) {
if (origin != SkEncodedOrigin::kDefault_SkEncodedOrigin) {
// The origin is applied after scaling, so use scaledSize, which is the final scaled size.
fMatrix = SkEncodedOriginToMatrix(origin, scaledSize.width(), scaledSize.height());
fCodec = SkAndroidCodec::MakeFromCodec(std::move(fCodec->fCodec),
SkAndroidCodec::ExifOrientationBehavior::kIgnore);
if (SkPixmapPriv::ShouldSwapWidthHeight(origin)) {
if (SkEncodedOriginSwapsWidthHeight(origin)) {
// The client asked for sizes post-rotation. Swap back to the pre-rotation sizes to pass
// to fCodec and for the scale matrix computation.
fDecodeInfo = SkPixmapPriv::SwapWidthHeight(fDecodeInfo);

View File

@ -60,7 +60,7 @@ static inline SkImageInfo adjust_info(SkCodec* codec,
SkAndroidCodec::ExifOrientationBehavior orientationBehavior) {
auto info = codec->getInfo();
if (orientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
|| !SkPixmapPriv::ShouldSwapWidthHeight(codec->getOrigin())) {
|| !SkEncodedOriginSwapsWidthHeight(codec->getOrigin())) {
return info;
}
return SkPixmapPriv::SwapWidthHeight(info);
@ -302,7 +302,7 @@ SkISize SkAndroidCodec::getSampledDimensions(int sampleSize) const {
auto dims = this->onGetSampledDimensions(sampleSize);
if (fOrientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
|| !SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
|| !SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
return dims;
}
@ -365,7 +365,7 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
SkImageInfo adjustedInfo = fInfo;
if (ExifOrientationBehavior::kRespect == fOrientationBehavior
&& SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
&& SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
adjustedInfo = SkPixmapPriv::SwapWidthHeight(adjustedInfo);
}

View File

@ -30,7 +30,7 @@ static SkImageInfo adjust_info(SkCodec* codec) {
if (kUnpremul_SkAlphaType == info.alphaType()) {
info = info.makeAlphaType(kPremul_SkAlphaType);
}
if (SkPixmapPriv::ShouldSwapWidthHeight(codec->getOrigin())) {
if (SkEncodedOriginSwapsWidthHeight(codec->getOrigin())) {
info = SkPixmapPriv::SwapWidthHeight(info);
}
return info;
@ -88,7 +88,7 @@ bool SkCodecImageGenerator::onGetYUVAPlanes(const SkYUVAPixmaps& yuvaPixmaps) {
SkISize SkCodecImageGenerator::getScaledDimensions(float desiredScale) const {
SkISize size = fCodec->getScaledDimensions(desiredScale);
if (SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
if (SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
std::swap(size.fWidth, size.fHeight);
}
return size;

View File

@ -583,7 +583,7 @@ bool SkPixmapPriv::Orient(const SkPixmap& dst, const SkPixmap& src, SkEncodedOri
int w = src.width();
int h = src.height();
if (ShouldSwapWidthHeight(origin)) {
if (SkEncodedOriginSwapsWidthHeight(origin)) {
using std::swap;
swap(w, h);
}
@ -601,11 +601,6 @@ bool SkPixmapPriv::Orient(const SkPixmap& dst, const SkPixmap& src, SkEncodedOri
return draw_orientation(dst, src, origin);
}
bool SkPixmapPriv::ShouldSwapWidthHeight(SkEncodedOrigin origin) {
// The last four SkEncodedOrigin values involve 90 degree rotations
return origin >= kLeftTop_SkEncodedOrigin;
}
SkImageInfo SkPixmapPriv::SwapWidthHeight(const SkImageInfo& info) {
return info.makeWH(info.height(), info.width());
}

View File

@ -20,7 +20,6 @@ public:
*/
static bool Orient(const SkPixmap& dst, const SkPixmap& src, SkEncodedOrigin);
static bool ShouldSwapWidthHeight(SkEncodedOrigin o);
static SkImageInfo SwapWidthHeight(const SkImageInfo& info);
/**
@ -39,7 +38,7 @@ public:
const SkPixmap* tmp = &dst;
if (origin != kTopLeft_SkEncodedOrigin) {
auto info = dst.info();
if (ShouldSwapWidthHeight(origin)) {
if (SkEncodedOriginSwapsWidthHeight(origin)) {
info = SwapWidthHeight(info);
}
if (!storage.tryAlloc(info)) {

View File

@ -92,7 +92,7 @@ std::unique_ptr<SkImageGenerator> SkImageGeneratorCG::MakeFromEncodedCG(sk_sp<Sk
origin = (SkEncodedOrigin) originInt;
}
if (SkPixmapPriv::ShouldSwapWidthHeight(origin)) {
if (SkEncodedOriginSwapsWidthHeight(origin)) {
info = SkPixmapPriv::SwapWidthHeight(info);
}

View File

@ -43,7 +43,7 @@ SkISize SkAnimCodecPlayer::dimensions() const {
auto image = fImages.front();
return image ? image->dimensions() : SkISize::MakeEmpty();
}
if (SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
if (SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
return { fImageInfo.height(), fImageInfo.width() };
}
return { fImageInfo.width(), fImageInfo.height() };

View File

@ -232,7 +232,7 @@ DEF_TEST(AndroidCodec_orientation, r) {
}
// SkAndroidCodec does not adjust for the origin by default. Dimensions may be reversed.
if (SkPixmapPriv::ShouldSwapWidthHeight(androidCodec->codec()->getOrigin())) {
if (SkEncodedOriginSwapsWidthHeight(androidCodec->codec()->getOrigin())) {
auto swappedDims = SkPixmapPriv::SwapWidthHeight(androidCodec->getInfo()).dimensions();
REPORTER_ASSERT(r, expectedDims == swappedDims);
} else {

View File

@ -29,6 +29,25 @@
#include <utility>
#include <vector>
DEF_TEST(AnimatedImage_simple, r) {
if (GetResourcePath().isEmpty()) {
return;
}
const char* file = "images/stoplight_h.webp";
auto data = GetResourceAsData(file);
if (!data) {
ERRORF(r, "Could not get %s", file);
return;
}
// An animated image with a non-default exif orientation is no longer
// "simple"; verify that the assert has been removed.
auto androidCodec = SkAndroidCodec::MakeFromData(std::move(data));
auto animatedImage = SkAnimatedImage::Make(std::move(androidCodec));
REPORTER_ASSERT(r, animatedImage);
}
DEF_TEST(AnimatedImage_invalidCrop, r) {
if (GetResourcePath().isEmpty()) {
return;