From 949c6006cbffee4afbe01a4e180299abafc7c4b9 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Thu, 23 Jan 2020 16:20:39 -0500 Subject: [PATCH] Make bitmap compression retain the ColorSpace Bug: 147877556 Test: I72b5f28903a67445bc205cdad8848384ad675b31 In Bitmap#compress and AndroidBitmap_compress, remove the conversion from F16 to 8888 P3. This was originally done because F16 forced a particular ColorSpace, and we wanted to use a wider gamut than SRGB. Now that F16 can be any RGB ColorSpace, encode it with the ColorSpace that it already has. Skip the conversion to 8888, which is unnecessary. The encoders can already convert from F16. Remove Bitmap::CompressResult. Now that compress does not allocate a Bitmap, there is no reason to return ALLOCATION_FAILED. Change-Id: I8190664398e762daf57092d919f382a56267dd07 --- core/jni/android/graphics/Bitmap.cpp | 3 +- .../android/graphics/apex/android_bitmap.cpp | 10 ++---- libs/hwui/hwui/Bitmap.cpp | 34 +++---------------- libs/hwui/hwui/Bitmap.h | 12 ++----- 4 files changed, 11 insertions(+), 48 deletions(-) diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp index 32a7cf32bc0ae..eb7d432b559be 100755 --- a/core/jni/android/graphics/Bitmap.cpp +++ b/core/jni/android/graphics/Bitmap.cpp @@ -475,8 +475,7 @@ static jboolean Bitmap_compress(JNIEnv* env, jobject clazz, jlong bitmapHandle, } auto fm = static_cast(format); - auto result = bitmap->bitmap().compress(fm, quality, strm.get()); - return result == Bitmap::CompressResult::Success ? JNI_TRUE : JNI_FALSE; + return bitmap->bitmap().compress(fm, quality, strm.get()) ? JNI_TRUE : JNI_FALSE; } static inline void bitmapErase(SkBitmap bitmap, const SkColor4f& color, diff --git a/core/jni/android/graphics/apex/android_bitmap.cpp b/core/jni/android/graphics/apex/android_bitmap.cpp index 789d02978c241..1a48af397ecf7 100644 --- a/core/jni/android/graphics/apex/android_bitmap.cpp +++ b/core/jni/android/graphics/apex/android_bitmap.cpp @@ -290,14 +290,8 @@ int ABitmap_compress(const AndroidBitmapInfo* info, ADataSpace dataSpace, const } CompressWriter stream(userContext, fn); - switch (Bitmap::compress(bitmap, format, quality, &stream)) { - case Bitmap::CompressResult::Success: - return ANDROID_BITMAP_RESULT_SUCCESS; - case Bitmap::CompressResult::AllocationFailed: - return ANDROID_BITMAP_RESULT_ALLOCATION_FAILED; - case Bitmap::CompressResult::Error: - return ANDROID_BITMAP_RESULT_JNI_EXCEPTION; - } + return Bitmap::compress(bitmap, format, quality, &stream) ? ANDROID_BITMAP_RESULT_SUCCESS + : ANDROID_BITMAP_RESULT_JNI_EXCEPTION; } AHardwareBuffer* ABitmap_getHardwareBuffer(ABitmap* bitmapHandle) { diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp index 3c402e9962185..7c0efe16838e1 100644 --- a/libs/hwui/hwui/Bitmap.cpp +++ b/libs/hwui/hwui/Bitmap.cpp @@ -471,36 +471,14 @@ BitmapPalette Bitmap::computePalette(const SkImageInfo& info, const void* addr, return BitmapPalette::Unknown; } -Bitmap::CompressResult Bitmap::compress(JavaCompressFormat format, int32_t quality, - SkWStream* stream) { +bool Bitmap::compress(JavaCompressFormat format, int32_t quality, SkWStream* stream) { SkBitmap skbitmap; getSkBitmap(&skbitmap); return compress(skbitmap, format, quality, stream); } -Bitmap::CompressResult Bitmap::compress(const SkBitmap& bitmap, JavaCompressFormat format, - int32_t quality, SkWStream* stream) { - SkBitmap skbitmap = bitmap; - if (skbitmap.colorType() == kRGBA_F16_SkColorType) { - // Convert to P3 before encoding. This matches - // SkAndroidCodec::computeOutputColorSpace for wide gamuts. Now that F16 - // could already be P3, we still want to convert to 8888. - auto cs = SkColorSpace::MakeRGB(SkNamedTransferFn::kSRGB, SkNamedGamut::kDCIP3); - auto info = skbitmap.info().makeColorType(kRGBA_8888_SkColorType) - .makeColorSpace(std::move(cs)); - SkBitmap p3; - if (!p3.tryAllocPixels(info)) { - return CompressResult::AllocationFailed; - } - - SkPixmap pm; - SkAssertResult(p3.peekPixels(&pm)); // should always work if tryAllocPixels() did. - if (!skbitmap.readPixels(pm)) { - return CompressResult::Error; - } - skbitmap = p3; - } - +bool Bitmap::compress(const SkBitmap& bitmap, JavaCompressFormat format, + int32_t quality, SkWStream* stream) { SkEncodedImageFormat fm; switch (format) { case JavaCompressFormat::Jpeg: @@ -518,12 +496,10 @@ Bitmap::CompressResult Bitmap::compress(const SkBitmap& bitmap, JavaCompressForm options.fQuality = quality; options.fCompression = format == JavaCompressFormat::WebpLossy ? SkWebpEncoder::Compression::kLossy : SkWebpEncoder::Compression::kLossless; - return SkWebpEncoder::Encode(stream, skbitmap.pixmap(), options) - ? CompressResult::Success : CompressResult::Error; + return SkWebpEncoder::Encode(stream, bitmap.pixmap(), options); } } - return SkEncodeImage(stream, skbitmap, fm, quality) - ? CompressResult::Success : CompressResult::Error; + return SkEncodeImage(stream, bitmap, fm, quality); } } // namespace android diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h index ee365af2f7be2..3bfb7800f735d 100644 --- a/libs/hwui/hwui/Bitmap.h +++ b/libs/hwui/hwui/Bitmap.h @@ -154,16 +154,10 @@ public: WebpLossless = 4, }; - enum class CompressResult { - Success, - AllocationFailed, - Error, - }; + bool compress(JavaCompressFormat format, int32_t quality, SkWStream* stream); - CompressResult compress(JavaCompressFormat format, int32_t quality, SkWStream* stream); - - static CompressResult compress(const SkBitmap& bitmap, JavaCompressFormat format, - int32_t quality, SkWStream* stream); + static bool compress(const SkBitmap& bitmap, JavaCompressFormat format, + int32_t quality, SkWStream* stream); private: static sk_sp allocateAshmemBitmap(size_t size, const SkImageInfo& i, size_t rowBytes); static sk_sp allocateHeapBitmap(size_t size, const SkImageInfo& i, size_t rowBytes);