Skip to content

Commit

Permalink
More efficient decoding for down-sampled Flutter images using cache(W…
Browse files Browse the repository at this point in the history
…idth|Height) (#15372)

When down-scaling images, decode encoded images into smaller images
closer to the target size before finally down-scaling them to their
target size. For very large images this can avoid inflating the image
into its full size first before throwing it away. This can help to
significantly reduce peak memory utilization.

On a tangent, we could be even more efficient, if we'd interpret the
cache(Width|Height) as sizing hints.

I also opportunistically added warnings, I don't think a "caching" API
should support scaling images up or changing their aspect ratio.
  • Loading branch information
ignatz authored and cbracken committed Jan 16, 2020
1 parent e0fe834 commit c7d0fb7
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 28 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Expand Up @@ -272,6 +272,7 @@ FILE: ../../../flutter/lib/ui/dart_ui.h
FILE: ../../../flutter/lib/ui/dart_wrapper.h
FILE: ../../../flutter/lib/ui/fixtures/DashInNooglerHat.jpg
FILE: ../../../flutter/lib/ui/fixtures/Horizontal.jpg
FILE: ../../../flutter/lib/ui/fixtures/Horizontal.png
FILE: ../../../flutter/lib/ui/fixtures/hello_loop_2.gif
FILE: ../../../flutter/lib/ui/fixtures/hello_loop_2.webp
FILE: ../../../flutter/lib/ui/fixtures/ui_test.dart
Expand Down
1 change: 1 addition & 0 deletions lib/ui/BUILD.gn
Expand Up @@ -155,6 +155,7 @@ if (current_toolchain == host_toolchain) {
fixtures = [
"fixtures/DashInNooglerHat.jpg",
"fixtures/Horizontal.jpg",
"fixtures/Horizontal.png",
"fixtures/hello_loop_2.gif",
"fixtures/hello_loop_2.webp",
]
Expand Down
Binary file added lib/ui/fixtures/Horizontal.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
151 changes: 123 additions & 28 deletions lib/ui/painting/image_decoder.cc
Expand Up @@ -4,11 +4,18 @@

#include "flutter/lib/ui/painting/image_decoder.h"

#include <algorithm>

#include "flutter/fml/make_copyable.h"
#include "flutter/fml/trace_event.h"
#include "third_party/skia/include/codec/SkCodec.h"
#include "third_party/skia/src/codec/SkCodecImageGenerator.h"

namespace flutter {
namespace {

constexpr double kAspectRatioChangedThreshold = 0.01;

} // namespace

ImageDecoder::ImageDecoder(
TaskRunners runners,
Expand All @@ -25,6 +32,10 @@ ImageDecoder::ImageDecoder(

ImageDecoder::~ImageDecoder() = default;

static double AspectRatio(const SkISize& size) {
return static_cast<double>(size.width()) / size.height();
}

// Get the updated dimensions of the image. If both dimensions are specified,
// use them. If one of them is specified, respect the one that is and use the
// aspect ratio to calculate the other. If neither dimension is specified, use
Expand All @@ -40,8 +51,7 @@ static SkISize GetResizedDimensions(SkISize current_size,
return SkISize::Make(target_width.value(), target_height.value());
}

const auto aspect_ratio =
static_cast<double>(current_size.width()) / current_size.height();
const auto aspect_ratio = AspectRatio(current_size);

if (target_width) {
return SkISize::Make(target_width.value(),
Expand All @@ -57,34 +67,52 @@ static SkISize GetResizedDimensions(SkISize current_size,
}

static sk_sp<SkImage> ResizeRasterImage(sk_sp<SkImage> image,
std::optional<uint32_t> target_width,
std::optional<uint32_t> target_height,
const SkISize& resized_dimensions,
const fml::tracing::TraceFlow& flow) {
FML_DCHECK(!image->isTextureBacked());

const auto resized_dimensions =
GetResizedDimensions(image->dimensions(), target_width, target_height);
TRACE_EVENT0("flutter", __FUNCTION__);
flow.Step(__FUNCTION__);

if (resized_dimensions.isEmpty()) {
FML_LOG(ERROR) << "Could not resize to empty dimensions.";
return nullptr;
}

if (resized_dimensions == image->dimensions()) {
// The resized dimesions are the same as the intrinsic dimensions of the
// image. There is nothing to do.
return image;
if (image->dimensions() == resized_dimensions) {
return image->makeRasterImage();
}

TRACE_EVENT0("flutter", __FUNCTION__);
flow.Step(__FUNCTION__);
if (resized_dimensions.width() > image->dimensions().width() ||
resized_dimensions.height() > image->dimensions().height()) {
FML_LOG(WARNING) << "Image is being upsized from "
<< image->dimensions().width() << "x"
<< image->dimensions().height() << " to "
<< resized_dimensions.width() << "x"
<< resized_dimensions.height()
<< ". Are cache(Height|Width) used correctly?";
// TOOD(48885): consider exiting here, there's no good reason to support
// upsampling in a "caching"-optimization context..
}

const auto scaled_image_info = image->imageInfo().makeWH(
resized_dimensions.width(), resized_dimensions.height());
const bool aspect_ratio_changed =
std::abs(AspectRatio(resized_dimensions) -
AspectRatio(image->dimensions())) > kAspectRatioChangedThreshold;
if (aspect_ratio_changed) {
// This is probably a bug. If a user passes dimensions that change the
// aspect ratio in a "caching" context that's probably not working as
// intended and rather a signal that the API is hard to use.
FML_LOG(WARNING)
<< "Aspect ratio changes. Are cache(Height|Width) used correctly?";
}

const auto scaled_image_info =
image->imageInfo().makeDimensions(resized_dimensions);

SkBitmap scaled_bitmap;
if (!scaled_bitmap.tryAllocPixels(scaled_image_info)) {
FML_LOG(ERROR) << "Could not allocate bitmap when attempting to scale.";
FML_LOG(ERROR) << "Failed to allocate memory for bitmap of size "
<< scaled_image_info.computeMinByteSize() << "B";
return nullptr;
}

Expand Down Expand Up @@ -122,31 +150,98 @@ static sk_sp<SkImage> ImageFromDecompressedData(
return nullptr;
}

return ResizeRasterImage(std::move(image), target_width, target_height, flow);
if (!target_width && !target_height) {
// No resizing requested. Just rasterize the image.
return image->makeRasterImage();
}

auto resized_dimensions =
GetResizedDimensions(image->dimensions(), target_width, target_height);

return ResizeRasterImage(std::move(image), resized_dimensions, flow);
}

static sk_sp<SkImage> ImageFromCompressedData(
sk_sp<SkData> data,
std::optional<uint32_t> target_width,
std::optional<uint32_t> target_height,
const fml::tracing::TraceFlow& flow) {
sk_sp<SkImage> ImageFromCompressedData(sk_sp<SkData> data,
std::optional<uint32_t> target_width,
std::optional<uint32_t> target_height,
const fml::tracing::TraceFlow& flow) {
TRACE_EVENT0("flutter", __FUNCTION__);
flow.Step(__FUNCTION__);

auto decoded_image = SkImage::MakeFromEncoded(data);
if (!target_width && !target_height) {
// No resizing requested. Just decode & rasterize the image.
return SkImage::MakeFromEncoded(data)->makeRasterImage();
}

if (!decoded_image) {
auto codec = SkCodec::MakeFromData(data);
if (codec == nullptr) {
return nullptr;
}

// Make sure to resolve all lazy images.
decoded_image = decoded_image->makeRasterImage();
const auto* codec_ptr = codec.get();

if (!decoded_image) {
// Note that we cannot read the dimensions from the codec since they don't
// respect image orientation provided e.g. in EXIF data.
auto image_generator = SkCodecImageGenerator::MakeFromCodec(std::move(codec));
const auto& source_dimensions = image_generator->getInfo().dimensions();

auto resized_dimensions =
GetResizedDimensions(source_dimensions, target_width, target_height);

// No resize needed.
if (resized_dimensions == source_dimensions) {
return SkImage::MakeFromEncoded(data)->makeRasterImage();
}

auto decode_dimensions = codec_ptr->getScaledDimensions(
std::max(static_cast<double>(resized_dimensions.width()) /
source_dimensions.width(),
static_cast<double>(resized_dimensions.height()) /
source_dimensions.height()));

// If the codec supports efficient sub-pixel decoding, decoded at a resolution
// close to the target resolution before resizing.
if (decode_dimensions != codec_ptr->dimensions()) {
if (source_dimensions != codec_ptr->dimensions()) {
decode_dimensions =
SkISize::Make(decode_dimensions.height(), decode_dimensions.width());
}

auto scaled_image_info =
image_generator->getInfo().makeDimensions(decode_dimensions);

SkBitmap scaled_bitmap;
if (!scaled_bitmap.tryAllocPixels(scaled_image_info)) {
FML_LOG(ERROR) << "Failed to allocate memory for bitmap of size "
<< scaled_image_info.computeMinByteSize() << "B";
return nullptr;
}

const auto& pixmap = scaled_bitmap.pixmap();
if (image_generator->getPixels(pixmap.info(), pixmap.writable_addr(),
pixmap.rowBytes())) {
// Marking this as immutable makes the MakeFromBitmap call share
// the pixels instead of copying.
scaled_bitmap.setImmutable();

auto decoded_image = SkImage::MakeFromBitmap(scaled_bitmap);
FML_DCHECK(decoded_image);
if (!decoded_image) {
FML_LOG(ERROR)
<< "Could not create a scaled image from a scaled bitmap.";
return nullptr;
}
return ResizeRasterImage(std::move(decoded_image), resized_dimensions,
flow);
}
}

auto image = SkImage::MakeFromEncoded(data);
if (!image) {
return nullptr;
}

return ResizeRasterImage(decoded_image, target_width, target_height, flow);
return ResizeRasterImage(std::move(image), resized_dimensions, flow);
}

static SkiaGPUObject<SkImage> UploadRasterImage(
Expand Down
6 changes: 6 additions & 0 deletions lib/ui/painting/image_decoder.h
Expand Up @@ -13,6 +13,7 @@
#include "flutter/fml/concurrent_message_loop.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/trace_event.h"
#include "flutter/lib/ui/io_manager.h"
#include "third_party/skia/include/core/SkData.h"
#include "third_party/skia/include/core/SkImage.h"
Expand Down Expand Up @@ -68,6 +69,11 @@ class ImageDecoder {
FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoder);
};

sk_sp<SkImage> ImageFromCompressedData(sk_sp<SkData> data,
std::optional<uint32_t> target_width,
std::optional<uint32_t> target_height,
const fml::tracing::TraceFlow& flow);

} // namespace flutter

#endif // FLUTTER_LIB_UI_PAINTING_IMAGE_DECODER_H_
38 changes: 38 additions & 0 deletions lib/ui/painting/image_decoder_unittests.cc
Expand Up @@ -519,5 +519,43 @@ TEST(ImageDecoderTest,
ASSERT_EQ(webp_codec->getRepetitionCount(), 1);
}

TEST(ImageDecoderTest, VerifySimpleDecoding) {
auto data = OpenFixtureAsSkData("Horizontal.jpg");
auto image = SkImage::MakeFromEncoded(data);
ASSERT_TRUE(image != nullptr);
ASSERT_EQ(SkISize::Make(600, 200), image->dimensions());

ASSERT_EQ(ImageFromCompressedData(data, 6, 2, fml::tracing::TraceFlow(""))
->dimensions(),
SkISize::Make(6, 2));
}

TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) {
auto data = OpenFixtureAsSkData("Horizontal.jpg");
auto image = SkImage::MakeFromEncoded(data);
ASSERT_TRUE(image != nullptr);
ASSERT_EQ(SkISize::Make(600, 200), image->dimensions());

auto decode = [data](std::optional<uint32_t> target_width,
std::optional<uint32_t> target_height) {
return ImageFromCompressedData(data, target_width, target_height,
fml::tracing::TraceFlow(""));
};

auto expected_data = OpenFixtureAsSkData("Horizontal.png");
ASSERT_TRUE(expected_data != nullptr);
ASSERT_FALSE(expected_data->isEmpty());

auto assert_image = [&](auto decoded_image) {
ASSERT_EQ(decoded_image->dimensions(), SkISize::Make(300, 100));
ASSERT_TRUE(decoded_image->encodeToData(SkEncodedImageFormat::kPNG, 100)
->equals(expected_data.get()));
};

assert_image(decode(300, 100));
assert_image(decode(300, {}));
assert_image(decode({}, 100));
}

} // namespace testing
} // namespace flutter

0 comments on commit c7d0fb7

Please sign in to comment.