Fix seaming issue in BitmapRegionDecoder for gainmaps
Rounding discrepancies in the image codec infrastructure was allowing
gainmaps to be decoded with a 1px border on the edge, which introduces
visible seams when zooming into large images.
Fix this by adjusting how computing the desired gainmap subregion size,
by pretending we downsample first, then compute the subregion size based
on that.
Add a long paragraph explaining why we have to do this with an example,
because it's not entirely intuitive.
Also adjust how we poke at the gainmap resampling flag, since some CTS
tests run before RenderThread starts up and intializes the properties.
Bug: 371123308
Flag: com.android.graphics.hwui.flags.resample_gainmap_regions
Test: Decoding in Photos and Files
Test: GainmapTest
Change-Id: Id608c5838f7890d0304b040e13d7281aa879a75c
diff --git a/libs/hwui/Properties.cpp b/libs/hwui/Properties.cpp
index ae46a99..064cac2 100644
--- a/libs/hwui/Properties.cpp
+++ b/libs/hwui/Properties.cpp
@@ -113,7 +113,6 @@
bool Properties::clipSurfaceViews = false;
bool Properties::hdr10bitPlus = false;
bool Properties::skipTelemetry = false;
-bool Properties::resampleGainmapRegions = false;
bool Properties::queryGlobalPriority = false;
int Properties::timeoutMultiplier = 1;
@@ -190,8 +189,6 @@
clipSurfaceViews =
base::GetBoolProperty("debug.hwui.clip_surfaceviews", hwui_flags::clip_surfaceviews());
hdr10bitPlus = hwui_flags::hdr_10bit_plus();
- resampleGainmapRegions = base::GetBoolProperty("debug.hwui.resample_gainmap_regions",
- hwui_flags::resample_gainmap_regions());
queryGlobalPriority = hwui_flags::query_global_priority();
timeoutMultiplier = android::base::GetIntProperty("ro.hw_timeout_multiplier", 1);
@@ -288,5 +285,11 @@
return base::GetBoolProperty(PROPERTY_INITIALIZE_GL_ALWAYS, hwui_flags::initialize_gl_always());
}
+bool Properties::resampleGainmapRegions() {
+ static bool sResampleGainmapRegions = base::GetBoolProperty(
+ "debug.hwui.resample_gainmap_regions", hwui_flags::resample_gainmap_regions());
+ return sResampleGainmapRegions;
+}
+
} // namespace uirenderer
} // namespace android
diff --git a/libs/hwui/Properties.h b/libs/hwui/Properties.h
index 6f84796..db930f3 100644
--- a/libs/hwui/Properties.h
+++ b/libs/hwui/Properties.h
@@ -345,7 +345,6 @@
static bool clipSurfaceViews;
static bool hdr10bitPlus;
static bool skipTelemetry;
- static bool resampleGainmapRegions;
static bool queryGlobalPriority;
static int timeoutMultiplier;
@@ -381,6 +380,7 @@
static void setDrawingEnabled(bool enable);
static bool initializeGlAlways();
+ static bool resampleGainmapRegions();
private:
static StretchEffectBehavior stretchEffectBehavior;
diff --git a/libs/hwui/jni/BitmapRegionDecoder.cpp b/libs/hwui/jni/BitmapRegionDecoder.cpp
index 5ffd5b9..491066b 100644
--- a/libs/hwui/jni/BitmapRegionDecoder.cpp
+++ b/libs/hwui/jni/BitmapRegionDecoder.cpp
@@ -112,9 +112,7 @@
return false;
}
- // Round out the subset so that we decode a slightly larger region, in
- // case the subset has fractional components.
- SkIRect roundedSubset = desiredSubset.roundOut();
+ sampleSize = std::max(sampleSize, 1);
// Map the desired subset to the space of the decoded gainmap. The
// subset is repositioned relative to the resulting bitmap, and then
@@ -123,10 +121,51 @@
// for existing gainmap formats.
SkRect logicalSubset = desiredSubset.makeOffset(-std::floorf(desiredSubset.left()),
-std::floorf(desiredSubset.top()));
- logicalSubset.fLeft /= sampleSize;
- logicalSubset.fTop /= sampleSize;
- logicalSubset.fRight /= sampleSize;
- logicalSubset.fBottom /= sampleSize;
+ logicalSubset = scale(logicalSubset, 1.0f / sampleSize);
+
+ // Round out the subset so that we decode a slightly larger region, in
+ // case the subset has fractional components. When we round, we need to
+ // round the downsampled subset to avoid possibly rounding down by accident.
+ // Consider this concrete example if we round the desired subset directly:
+ //
+ // * We are decoding a 18x18 corner of an image
+ //
+ // * Gainmap is 1/4 resolution, which is logically a 4.5x4.5 gainmap
+ // that we would want
+ //
+ // * The app wants to downsample by a factor of 2x
+ //
+ // * The desired gainmap dimensions are computed to be 3x3 to fit the
+ // downsampled gainmap, since we need to fill a 2.25x2.25 region that's
+ // later upscaled to 3x3
+ //
+ // * But, if we round out the desired gainmap region _first_, then we
+ // request to decode a 5x5 region, downsampled by 2, which actually
+ // decodes a 2x2 region since skia rounds down internally. But then we transfer
+ // the result to a 3x3 bitmap using a clipping allocator, which leaves an inset.
+ // Not only did we get a smaller region than we expected (so, our desired subset is
+ // not valid), but because the API allows for decoding regions using a recycled
+ // bitmap, we can't really safely fill in the inset since then we might
+ // extend the gainmap beyond intended the image bounds. Oops.
+ //
+ // * If we instead round out as if we downsampled, then we downsample
+ // the desired region to 2.25x2.25, round out to 3x3, then upsample back
+ // into the source gainmap space to get 6x6. Then we decode a 6x6 region
+ // downsampled into a 3x3 region, and everything's now correct.
+ //
+ // Note that we don't always run into this problem, because
+ // decoders actually round *up* for subsampling when decoding a subset
+ // that matches the dimensions of the image. E.g., if the original image
+ // size in the above example was a 20x20 image, so that the gainmap was
+ // 5x5, then we still manage to downsample into a 3x3 bitmap even with
+ // the "wrong" math. but that's what we wanted!
+ //
+ // Note also that if we overshoot the gainmap bounds with the requested
+ // subset it isn't a problem either, since now the decoded bitmap is too
+ // large, rather than too small, so now we can use the desired subset to
+ // avoid sampling "invalid" colors.
+ SkRect scaledSubset = scale(desiredSubset, 1.0f / sampleSize);
+ SkIRect roundedSubset = scale(scaledSubset.roundOut(), static_cast<float>(sampleSize));
RecyclingClippingPixelAllocator allocator(nativeBitmap.get(), false, logicalSubset);
if (!mGainmapBRD->decodeRegion(&bm, &allocator, roundedSubset, sampleSize, decodeColorType,
@@ -154,7 +193,7 @@
const float scaleX = ((float)mGainmapBRD->width()) / mMainImageBRD->width();
const float scaleY = ((float)mGainmapBRD->height()) / mMainImageBRD->height();
- if (uirenderer::Properties::resampleGainmapRegions) {
+ if (uirenderer::Properties::resampleGainmapRegions()) {
const auto srcRect = SkRect::MakeLTRB(
mainImageRegion.left() * scaleX, mainImageRegion.top() * scaleY,
mainImageRegion.right() * scaleX, mainImageRegion.bottom() * scaleY);
@@ -186,6 +225,22 @@
, mGainmapBRD(std::move(gainmapBRD))
, mGainmapInfo(info) {}
+ SkRect scale(SkRect rect, float scale) const {
+ rect.fLeft *= scale;
+ rect.fTop *= scale;
+ rect.fRight *= scale;
+ rect.fBottom *= scale;
+ return rect;
+ }
+
+ SkIRect scale(SkIRect rect, float scale) const {
+ rect.fLeft *= scale;
+ rect.fTop *= scale;
+ rect.fRight *= scale;
+ rect.fBottom *= scale;
+ return rect;
+ }
+
std::unique_ptr<skia::BitmapRegionDecoder> mMainImageBRD;
std::unique_ptr<skia::BitmapRegionDecoder> mGainmapBRD;
SkGainmapInfo mGainmapInfo;
diff --git a/libs/hwui/jni/Graphics.cpp b/libs/hwui/jni/Graphics.cpp
index 258bf91..a210ddf 100644
--- a/libs/hwui/jni/Graphics.cpp
+++ b/libs/hwui/jni/Graphics.cpp
@@ -750,7 +750,7 @@
std::optional<SkRect> RecyclingClippingPixelAllocator::getSourceBoundsForUpsample(
std::optional<SkRect> subset) {
- if (!uirenderer::Properties::resampleGainmapRegions || !subset || subset->isEmpty()) {
+ if (!uirenderer::Properties::resampleGainmapRegions() || !subset || subset->isEmpty()) {
return std::nullopt;
}