AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Deprecate (and ignore) SkAndroidCodec::ExifOrientation

Browse Source 
This was originally added so that some clients (BitmapFactory,
BitmapRegionDecoder) could ignore exif for backwards compatibility, and
others (ImageDecoder) could respect it.

With the addition of NDK APIs for decoding all frames of an animated
image, hwui/ImageDecoder will handle compositing frames, including
handling the orientation, so it may as well always handle it.

This removes tests for SkAndroidCodec that are no longer applicable.
Android already has tests for most of them.
AndroidCodec_sampledOrientation is recreated in
ag/Ieda439910ae52e609f0710d424503616d99ae5c7.

Change-Id: Ibd280986892176f284895d543f2f50bca22d196b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344763
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
This commit is contained in:
Leon Scroggins III 2020-12-15 15:58:53 -05:00 committed by Skia Commit-Bot
parent ea485e5285
commit add35d9474
8 changed files with 41 additions and 270 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
RELEASE_NOTES.txt
View File

@ -7,7 +7,9 @@ This file includes a list of high level updates for each milestone release.
Milestone 90
------------
*
* Deprecate (and ignore) SkAndroidCodec::ExifOrientation
https://review.skia.org/344763
Milestone 89
------------

35
include/codec/SkAndroidCodec.h
View File

@ -19,31 +19,23 @@
*/
class SK_API SkAndroidCodec : SkNoncopyable {
public:
/**
* Deprecated.
*
* Now that SkAndroidCodec supports multiframe images, there are multiple
* ways to handle compositing an oriented frame on top of an oriented frame
* with different tradeoffs. SkAndroidCodec now ignores the orientation and
* forces the client to handle it.
*/
enum class ExifOrientationBehavior {
/**
* Ignore any exif orientation markers in the data.
*
* getInfo's width and height will match the header of the image, and
* no processing will be done to match the marker.
*/
kIgnore,
/**
* Respect the exif orientation marker.
*
* getInfo's width and height will represent what they should be after
* applying the orientation. For example, if the marker specifies a
* rotation by 90 degrees, they will be swapped relative to the header.
* getAndroidPixels will apply the orientation as well.
*/
kRespect,
};
/**
* Pass ownership of an SkCodec to a newly-created SkAndroidCodec.
*/
static std::unique_ptr<SkAndroidCodec> MakeFromCodec(std::unique_ptr<SkCodec>,
ExifOrientationBehavior = ExifOrientationBehavior::kIgnore);
static std::unique_ptr<SkAndroidCodec> MakeFromCodec(std::unique_ptr<SkCodec>);
/**
* If this stream represents an encoded image that we know how to decode,
@ -54,8 +46,6 @@ public:
*
* If NULL is returned, the stream is deleted immediately. Otherwise, the
* SkCodec takes ownership of it, and will delete it when done with it.
*
* ExifOrientationBehavior is set to kIgnore.
*/
static std::unique_ptr<SkAndroidCodec> MakeFromStream(std::unique_ptr<SkStream>,
SkPngChunkReader* = nullptr);
@ -66,13 +56,13 @@ public:
*
* The SkPngChunkReader handles unknown chunks in PNGs.
* See SkCodec.h for more details.
*
* ExifOrientationBehavior is set to kIgnore.
*/
static std::unique_ptr<SkAndroidCodec> MakeFromData(sk_sp<SkData>, SkPngChunkReader* = nullptr);
virtual ~SkAndroidCodec();
// TODO: fInfo is now just a cache of SkCodec's SkImageInfo. No need to
// cache and return a reference here, once Android call-sites are updated.
const SkImageInfo& getInfo() const { return fInfo; }
/**
@ -257,7 +247,7 @@ public:
SkCodec* codec() const { return fCodec.get(); }
protected:
SkAndroidCodec(SkCodec*, ExifOrientationBehavior = ExifOrientationBehavior::kIgnore);
SkAndroidCodec(SkCodec*);
virtual SkISize onGetSampledDimensions(int sampleSize) const = 0;
@ -268,7 +258,6 @@ protected:
private:
const SkImageInfo fInfo;
const ExifOrientationBehavior fOrientationBehavior;
std::unique_ptr<SkCodec> fCodec;
};
#endif // SkAndroidCodec_DEFINED

104
src/codec/SkAndroidCodec.cpp
View File

@ -11,7 +11,6 @@
#include "src/codec/SkAndroidCodecAdapter.h"
#include "src/codec/SkCodecPriv.h"
#include "src/codec/SkSampledCodec.h"
#include "src/core/SkPixmapPriv.h"
static bool is_valid_sample_size(int sampleSize) {
// FIXME: As Leon has mentioned elsewhere, surely there is also a maximum sampleSize?
@ -56,19 +55,8 @@ static bool is_wide_gamut(const skcms_ICCProfile& profile) {
return false;
}
static inline SkImageInfo adjust_info(SkCodec* codec,
SkAndroidCodec::ExifOrientationBehavior orientationBehavior) {
auto info = codec->getInfo();
if (orientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
|| !SkEncodedOriginSwapsWidthHeight(codec->getOrigin())) {
return info;
}
return SkPixmapPriv::SwapWidthHeight(info);
}
SkAndroidCodec::SkAndroidCodec(SkCodec* codec, ExifOrientationBehavior orientationBehavior)
: fInfo(adjust_info(codec, orientationBehavior))
, fOrientationBehavior(orientationBehavior)
SkAndroidCodec::SkAndroidCodec(SkCodec* codec)
: fInfo(codec->getInfo())
, fCodec(codec)
{}
@ -80,8 +68,7 @@ std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromStream(std::unique_ptr<S
return MakeFromCodec(std::move(codec));
}
std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec,
ExifOrientationBehavior orientationBehavior) {
std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec) {
if (nullptr == codec) {
return nullptr;
}
@ -97,7 +84,7 @@ std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<Sk
case SkEncodedImageFormat::kWBMP:
case SkEncodedImageFormat::kHEIF:
case SkEncodedImageFormat::kAVIF:
return std::make_unique<SkSampledCodec>(codec.release(), orientationBehavior);
return std::make_unique<SkSampledCodec>(codec.release());
#ifdef SK_HAS_WUFFS_LIBRARY
case SkEncodedImageFormat::kGIF:
#endif
@ -108,7 +95,7 @@ std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<Sk
case SkEncodedImageFormat::kDNG:
#endif
#if defined(SK_CODEC_DECODES_WEBP) || defined(SK_CODEC_DECODES_RAW) || defined(SK_HAS_WUFFS_LIBRARY)
return std::make_unique<SkAndroidCodecAdapter>(codec.release(), orientationBehavior);
return std::make_unique<SkAndroidCodecAdapter>(codec.release());
#endif
default:
@ -216,12 +203,13 @@ static inline bool strictly_bigger_than(const SkISize& a, const SkISize& b) {
int SkAndroidCodec::computeSampleSize(SkISize* desiredSize) const {
SkASSERT(desiredSize);
if (!desiredSize || *desiredSize == fInfo.dimensions()) {
const auto origDims = fCodec->dimensions();
if (!desiredSize || *desiredSize == origDims) {
return 1;
}
if (smaller_than(fInfo.dimensions(), *desiredSize)) {
*desiredSize = fInfo.dimensions();
if (smaller_than(origDims, *desiredSize)) {
*desiredSize = origDims;
return 1;
}
@ -235,15 +223,15 @@ int SkAndroidCodec::computeSampleSize(SkISize* desiredSize) const {
return 1;
}
int sampleX = fInfo.width() / desiredSize->width();
int sampleY = fInfo.height() / desiredSize->height();
int sampleX = origDims.width() / desiredSize->width();
int sampleY = origDims.height() / desiredSize->height();
int sampleSize = std::min(sampleX, sampleY);
auto computedSize = this->getSampledDimensions(sampleSize);
if (computedSize == *desiredSize) {
return sampleSize;
}
if (computedSize == fInfo.dimensions() || sampleSize == 1) {
if (computedSize == origDims || sampleSize == 1) {
// Cannot downscale
*desiredSize = computedSize;
return 1;
@ -286,7 +274,7 @@ int SkAndroidCodec::computeSampleSize(SkISize* desiredSize) const {
sampleSize--;
}
*desiredSize = fInfo.dimensions();
*desiredSize = origDims;
return 1;
}
@ -297,20 +285,14 @@ SkISize SkAndroidCodec::getSampledDimensions(int sampleSize) const {
// Fast path for when we are not scaling.
if (1 == sampleSize) {
return fInfo.dimensions();
return fCodec->dimensions();
}
auto dims = this->onGetSampledDimensions(sampleSize);
if (fOrientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
|| !SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
return dims;
}
return { dims.height(), dims.width() };
return this->onGetSampledDimensions(sampleSize);
}
bool SkAndroidCodec::getSupportedSubset(SkIRect* desiredSubset) const {
if (!desiredSubset || !is_valid_subset(*desiredSubset, fInfo.dimensions())) {
if (!desiredSubset || !is_valid_subset(*desiredSubset, fCodec->dimensions())) {
return false;
}
@ -331,7 +313,7 @@ SkISize SkAndroidCodec::getSampledSubsetDimensions(int sampleSize, const SkIRect
}
// If the subset is the entire image, for consistency, use getSampledDimensions().
if (fInfo.dimensions() == subset.size()) {
if (fCodec->dimensions() == subset.size()) {
return this->getSampledDimensions(sampleSize);
}
@ -341,19 +323,6 @@ SkISize SkAndroidCodec::getSampledSubsetDimensions(int sampleSize, const SkIRect
get_scaled_dimension(subset.height(), sampleSize)};
}
static bool acceptable_result(SkCodec::Result result) {
switch (result) {
// These results mean a partial or complete image. They should be considered
// a success by SkPixmapPriv.
case SkCodec::kSuccess:
case SkCodec::kIncompleteInput:
case SkCodec::kErrorInInput:
return true;
default:
return false;
}
}
SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
void* requestPixels, size_t requestRowBytes, const AndroidOptions* options) {
if (!requestPixels) {
@ -363,22 +332,16 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
return SkCodec::kInvalidParameters;
}
SkImageInfo adjustedInfo = fInfo;
if (ExifOrientationBehavior::kRespect == fOrientationBehavior
&& SkEncodedOriginSwapsWidthHeight(fCodec->getOrigin())) {
adjustedInfo = SkPixmapPriv::SwapWidthHeight(adjustedInfo);
}
AndroidOptions defaultOptions;
if (!options) {
options = &defaultOptions;
} else {
if (options->fSubset) {
if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) {
if (!is_valid_subset(*options->fSubset, fCodec->dimensions())) {
return SkCodec::kInvalidParameters;
}
if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) {
if (SkIRect::MakeSize(fCodec->dimensions()) == *options->fSubset) {
// The caller wants the whole thing, rather than a subset. Modify
// the AndroidOptions passed to onGetAndroidPixels to not specify
// a subset.
@ -387,13 +350,6 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
options = &defaultOptions;
}
}
// To simplify frame compositing, force the client to use kIgnore and
// handle orientation themselves.
if (options->fFrameIndex != 0 && fOrientationBehavior == ExifOrientationBehavior::kRespect
&& fCodec->getOrigin() != kDefault_SkEncodedOrigin) {
return SkCodec::kInvalidParameters;
}
}
if (auto result = fCodec->handleFrameIndex(requestInfo, requestPixels, requestRowBytes,
@ -401,27 +357,7 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
return result;
}
if (ExifOrientationBehavior::kIgnore == fOrientationBehavior) {
return this->onGetAndroidPixels(requestInfo, requestPixels, requestRowBytes, *options);
}
SkCodec::Result result;
auto decode = [this, options, &result](const SkPixmap& pm) {
result = this->onGetAndroidPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), *options);
return acceptable_result(result);
};
SkPixmap dst(requestInfo, requestPixels, requestRowBytes);
if (SkPixmapPriv::Orient(dst, fCodec->getOrigin(), decode)) {
return result;
}
// Orient returned false. If onGetAndroidPixels succeeded, then Orient failed internally.
if (acceptable_result(result)) {
return SkCodec::kInternalError;
}
return result;
return this->onGetAndroidPixels(requestInfo, requestPixels, requestRowBytes, *options);
}
SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,

4
src/codec/SkAndroidCodecAdapter.cpp
View File

@ -8,8 +8,8 @@
#include "src/codec/SkAndroidCodecAdapter.h"
#include "src/codec/SkCodecPriv.h"
SkAndroidCodecAdapter::SkAndroidCodecAdapter(SkCodec* codec, ExifOrientationBehavior behavior)
: INHERITED(codec, behavior)
SkAndroidCodecAdapter::SkAndroidCodecAdapter(SkCodec* codec)
: INHERITED(codec)
{}
SkISize SkAndroidCodecAdapter::onGetSampledDimensions(int sampleSize) const {

2
src/codec/SkAndroidCodecAdapter.h
View File

@ -17,7 +17,7 @@
class SkAndroidCodecAdapter : public SkAndroidCodec {
public:
explicit SkAndroidCodecAdapter(SkCodec*, ExifOrientationBehavior);
explicit SkAndroidCodecAdapter(SkCodec*);
~SkAndroidCodecAdapter() override {}

4
src/codec/SkSampledCodec.cpp
View File

@ -13,8 +13,8 @@
#include "src/codec/SkSampler.h"
#include "src/core/SkMathPriv.h"
SkSampledCodec::SkSampledCodec(SkCodec* codec, ExifOrientationBehavior behavior)
: INHERITED(codec, behavior)
SkSampledCodec::SkSampledCodec(SkCodec* codec)
: INHERITED(codec)
{}
SkISize SkSampledCodec::accountForNativeScaling(int* sampleSizePtr, int* nativeSampleSize) const {

2
src/codec/SkSampledCodec.h
View File

@ -16,7 +16,7 @@
*/
class SkSampledCodec : public SkAndroidCodec {
public:
explicit SkSampledCodec(SkCodec*, ExifOrientationBehavior);
explicit SkSampledCodec(SkCodec*);
~SkSampledCodec() override {}

156
tests/AndroidCodecTest.cpp
View File

@ -202,159 +202,3 @@ DEF_TEST(AndroidCodec_P3, r) {
}};
REPORTER_ASSERT(r, 0 == memcmp(&matrix, &kExpected, sizeof(skcms_Matrix3x3)));
}
DEF_TEST(AndroidCodec_orientation, r) {
if (GetResourcePath().isEmpty()) {
return;
}
for (const char* filePathFormatStr : {"images/orientation/%c_420.jpg",
"images/orientation/%c.webp"})
for (char i = '1'; i <= '8'; ++i) {
SkString path = SkStringPrintf(filePathFormatStr, i);
auto data = GetResourceAsData(path.c_str());
auto gen = SkCodecImageGenerator::MakeFromEncodedCodec(data);
if (!gen) {
ERRORF(r, "failed to decode %s", path.c_str());
return;
}
// Dimensions after adjusting for the origin.
const SkISize expectedDims = { 100, 80 };
// SkCodecImageGenerator automatically adjusts for the origin.
REPORTER_ASSERT(r, gen->getInfo().dimensions() == expectedDims);
auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(data));
if (!androidCodec) {
ERRORF(r, "failed to decode %s", path.c_str());
return;
}
// SkAndroidCodec does not adjust for the origin by default. Dimensions may be reversed.
if (SkEncodedOriginSwapsWidthHeight(androidCodec->codec()->getOrigin())) {
auto swappedDims = SkPixmapPriv::SwapWidthHeight(androidCodec->getInfo()).dimensions();
REPORTER_ASSERT(r, expectedDims == swappedDims);
} else {
REPORTER_ASSERT(r, expectedDims == androidCodec->getInfo().dimensions());
}
// Passing kRespect adjusts for the origin.
androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(std::move(data)),
SkAndroidCodec::ExifOrientationBehavior::kRespect);
auto info = androidCodec->getInfo();
REPORTER_ASSERT(r, info.dimensions() == expectedDims);
SkBitmap fromGenerator;
fromGenerator.allocPixels(info);
REPORTER_ASSERT(r, gen->getPixels(info, fromGenerator.getPixels(),
fromGenerator.rowBytes()));
SkBitmap fromAndroidCodec;
fromAndroidCodec.allocPixels(info);
auto result = androidCodec->getPixels(info, fromAndroidCodec.getPixels(),
fromAndroidCodec.rowBytes());
REPORTER_ASSERT(r, result == SkCodec::kSuccess);
for (int i = 0; i < info.width(); ++i)
for (int j = 0; j < info.height(); ++j) {
SkColor c1 = *fromGenerator .getAddr32(i, j);
SkColor c2 = *fromAndroidCodec.getAddr32(i, j);
if (c1 != c2) {
ERRORF(r, "Bitmaps for %s do not match starting at position %i, %i\n"
"\tfromGenerator: %x\tfromAndroidCodec: %x", path.c_str(), i, j,
c1, c2);
return;
}
}
}
}
DEF_TEST(AndroidCodec_sampledOrientation, r) {
if (GetResourcePath().isEmpty()) {
return;
}
// kRightTop_SkEncodedOrigin = 6, // Rotated 90 CW
auto path = "images/orientation/6_420.jpg";
auto data = GetResourceAsData(path);
if (!data) {
ERRORF(r, "Failed to get resource %s", path);
return;
}
auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(std::move(data)),
SkAndroidCodec::ExifOrientationBehavior::kRespect);
constexpr int sampleSize = 7;
auto sampledDims = androidCodec->getSampledDimensions(sampleSize);
SkAndroidCodec::AndroidOptions options;
options.fSampleSize = sampleSize;
SkBitmap bm;
auto info = androidCodec->getInfo().makeDimensions(sampledDims);
bm.allocPixels(info);
auto result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes(), &options);
if (result != SkCodec::kSuccess) {
ERRORF(r, "got result \"%s\"\n", SkCodec::ResultToString(result));
}
}
DEF_TEST(AndroidCodec_animatedOrientation, r) {
if (GetResourcePath().isEmpty()) {
return;
}
static const struct {
const char* path;
SkEncodedOrigin origin;
} gRec[] = {
{ "images/stoplight.webp", kDefault_SkEncodedOrigin },
{ "images/stoplight_h.webp", kLeftBottom_SkEncodedOrigin } // Rotated 90 CCW
};
for (auto rec : gRec) {
auto data = GetResourceAsData(rec.path);
if (!data) {
ERRORF(r, "Failed to get resource %s", rec.path);
return;
}
// SkAndroidCodec now allows decoding frames beyond the first, but combining this with
// kRespect-ing a non-kDefault_SkEncodedOrigin is not supported. If a frame depends on
// a prior frame, we allow a client to provide that prior frame. But in order to respect
// the origin, we would need to transform the prior frame back to the original orientation
// in order to blend (and potentially erase, for a kRestoreBG frame) just to transform it
// back. Instead, force the client to handle the orientation after the fact.
for (auto behavior : { SkAndroidCodec::ExifOrientationBehavior::kRespect,
SkAndroidCodec::ExifOrientationBehavior::kIgnore }) {
auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(data),
behavior);
REPORTER_ASSERT(r, androidCodec->codec()->getOrigin() == rec.origin);
auto info = androidCodec->getInfo();
SkBitmap bm;
bm.allocPixels(info);
auto result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes());
REPORTER_ASSERT(r, result == SkCodec::kSuccess);
SkAndroidCodec::AndroidOptions options;
options.fFrameIndex = 1;
options.fPriorFrame = 0;
result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes(), &options);
switch (behavior) {
case SkAndroidCodec::ExifOrientationBehavior::kRespect:
if (rec.origin != kDefault_SkEncodedOrigin) {
REPORTER_ASSERT(r, result == SkCodec::kInvalidParameters,
"Should not be able to decode frame 1 with exif orientation"
" directly! Result: %s", SkCodec::ResultToString(result));
break;
}
[[fallthrough]];
case SkAndroidCodec::ExifOrientationBehavior::kIgnore:
REPORTER_ASSERT(r, result == SkCodec::kSuccess);
break;
}
}
}
}