Reland "Make SkSurface::asyncRescaleAndRead use kStrict constraint"
This is a reland of 451b01fe098faa18f6552ff77e5e1170a0f525c4
Original change's description:
> Make SkSurface::asyncRescaleAndRead use kStrict constraint
>
> Also fix crash if SkSurface has no color space.
>
> Bug: skia:8962
> Change-Id: I5826ddb10daa46f286d2405b7229ea4f0aefa8f2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214306
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:8962
Change-Id: I05713351f62cc738ec8a4bba6377e9aa7a044975
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/214684
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/gm/asyncrescaleandread.cpp b/gm/asyncrescaleandread.cpp
index 32e8eb3..e539fe9 100644
--- a/gm/asyncrescaleandread.cpp
+++ b/gm/asyncrescaleandread.cpp
@@ -19,20 +19,13 @@
// Draws the image to a surface, does a asyncRescaleAndReadPixels of the image, and then sticks
// the result in a raster image.
-static sk_sp<SkImage> do_read_and_scale(
- SkImage* image, const SkIRect& srcRect, const SkImageInfo& ii,
- SkSurface::RescaleGamma rescaleGamma, SkFilterQuality quality,
- std::function<sk_sp<SkSurface>(const SkImageInfo&)> makeSurface) {
+static sk_sp<SkImage> do_read_and_scale(SkSurface* surface, const SkIRect& srcRect,
+ const SkImageInfo& ii, SkSurface::RescaleGamma rescaleGamma,
+ SkFilterQuality quality) {
SkBitmap bmp;
bmp.allocPixels(ii);
- // Turn the image into a surface in order to call the read and rescale API
- auto surf = makeSurface(image->imageInfo().makeWH(image->width(), image->height()));
- if (!surf) {
- return nullptr;
- }
SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc);
- surf->getCanvas()->drawImage(image, 0, 0, &paint);
struct Context {
SkPixmap fPixmap;
bool fCalled = false;
@@ -48,52 +41,74 @@
SkRectMemcpy(context->fPixmap.writable_addr(), context->fPixmap.rowBytes(), data, rowBytes,
context->fPixmap.info().minRowBytes(), context->fPixmap.height());
};
- surf->asyncRescaleAndReadPixels(ii, srcRect, rescaleGamma, quality, callback, &context);
+ surface->asyncRescaleAndReadPixels(ii, srcRect, rescaleGamma, quality, callback, &context);
while (!context.fCalled) {
// Only GPU should actually be asynchronous.
- SkASSERT(surf->getCanvas()->getGrContext());
- surf->getCanvas()->getGrContext()->checkAsyncWorkCompletion();
+ SkASSERT(surface->getCanvas()->getGrContext());
+ surface->getCanvas()->getGrContext()->checkAsyncWorkCompletion();
}
return SkImage::MakeFromBitmap(bmp);
}
// Draws a grid of rescales. The columns are none, low, and high filter quality. The rows are
// rescale in src gamma and rescale in linear gamma.
-static skiagm::DrawResult do_rescale_grid(SkCanvas* canvas, const char* imageFile,
+static skiagm::DrawResult do_rescale_grid(SkCanvas* canvas, SkSurface* surface,
const SkIRect& srcRect, int newW, int newH,
- SkString* errorMsg) {
+ SkString* errorMsg, int pad = 0) {
if (canvas->imageInfo().colorType() == kUnknown_SkColorType) {
*errorMsg = "Not supported on recording/vector backends.";
return skiagm::DrawResult::kSkip;
}
- auto image = GetResourceAsImage(imageFile);
- if (!image) {
- errorMsg->printf("Could not load image file %s.", imageFile);
- return skiagm::DrawResult::kFail;
- }
const auto ii = canvas->imageInfo().makeWH(newW, newH);
- auto makeSurface = [canvas](const SkImageInfo& info) { return canvas->makeSurface(info); };
canvas->save();
for (auto linear : {SkSurface::RescaleGamma::kSrc, SkSurface::RescaleGamma::kLinear}) {
canvas->save();
for (auto quality : {kNone_SkFilterQuality, kLow_SkFilterQuality, kHigh_SkFilterQuality}) {
- auto rescaled =
- do_read_and_scale(image.get(), srcRect, ii, linear, quality, makeSurface);
+ auto rescaled = do_read_and_scale(surface, srcRect, ii, linear, quality);
canvas->drawImage(rescaled, 0, 0);
- canvas->translate(newW, 0);
+ canvas->translate(newW + pad, 0);
}
canvas->restore();
- canvas->translate(0, newH);
+ canvas->translate(0, newH + pad);
}
canvas->restore();
return skiagm::DrawResult::kOk;
}
+static skiagm::DrawResult do_rescale_image_grid(SkCanvas* canvas, const char* imageFile,
+ const SkIRect& srcRect, int newW, int newH,
+ SkString* errorMsg) {
+ auto image = GetResourceAsImage(imageFile);
+ if (!image) {
+ errorMsg->printf("Could not load image file %s.", imageFile);
+ return skiagm::DrawResult::kFail;
+ }
+ if (canvas->imageInfo().colorType() == kUnknown_SkColorType) {
+ *errorMsg = "Not supported on recording/vector backends.";
+ return skiagm::DrawResult::kSkip;
+ }
+ // Turn the image into a surface in order to call the read and rescale API
+ auto surfInfo = image->imageInfo().makeWH(image->width(), image->height());
+ auto surface = canvas->makeSurface(surfInfo);
+ if (!surface && surfInfo.colorType() == kBGRA_8888_SkColorType) {
+ surfInfo = surfInfo.makeColorType(kRGBA_8888_SkColorType);
+ surface = canvas->makeSurface(surfInfo);
+ }
+ if (!surface) {
+ *errorMsg = "Could not create surface for image.";
+ return skiagm::DrawResult::kFail;
+ }
+ SkPaint paint;
+ paint.setBlendMode(SkBlendMode::kSrc);
+ surface->getCanvas()->drawImage(image, 0, 0, &paint);
+ return do_rescale_grid(canvas, surface.get(), srcRect, newW, newH, errorMsg);
+}
+
#define DEF_RESCALE_AND_READ_GM(IMAGE_FILE, TAG, SRC_RECT, W, H) \
DEF_SIMPLE_GM_CAN_FAIL(async_rescale_and_read_##TAG, canvas, errorMsg, 3 * W, 2 * H) { \
ToolUtils::draw_checkerboard(canvas, SK_ColorDKGRAY, SK_ColorLTGRAY, 25); \
- return do_rescale_grid(canvas, #IMAGE_FILE, SRC_RECT, W, H, errorMsg); \
+ return do_rescale_image_grid(canvas, #IMAGE_FILE, SRC_RECT, W, H, errorMsg); \
}
DEF_RESCALE_AND_READ_GM(images/yellow_rose.webp, rose, SkIRect::MakeXYWH(100, 20, 100, 100),
@@ -108,3 +123,40 @@
(int)(1.2 * 105))
DEF_RESCALE_AND_READ_GM(images/text.png, text_up_large, SkIRect::MakeXYWH(300, 0, 300, 105),
(int)(2.4 * 300), (int)(2.4 * 105))
+
+DEF_SIMPLE_GM_CAN_FAIL(async_rescale_and_read_no_bleed, canvas, errorMsg, 60, 60) {
+ if (canvas->imageInfo().colorType() == kUnknown_SkColorType) {
+ *errorMsg = "Not supported on recording/vector backends.";
+ return skiagm::DrawResult::kSkip;
+ }
+
+ static constexpr int kBorder = 5;
+ static constexpr int kInner = 5;
+ const auto srcRect = SkIRect::MakeXYWH(kBorder, kBorder, kInner, kInner);
+ auto surfaceII =
+ SkImageInfo::Make(kInner + 2 * kBorder, kInner + 2 * kBorder, kRGBA_8888_SkColorType,
+ kPremul_SkAlphaType, SkColorSpace::MakeSRGB());
+ auto surface = canvas->makeSurface(surfaceII);
+ surface->getCanvas()->clear(SK_ColorRED);
+ surface->getCanvas()->save();
+ surface->getCanvas()->clipRect(SkRect::Make(srcRect), SkClipOp::kIntersect, false);
+ surface->getCanvas()->clear(SK_ColorBLUE);
+ surface->getCanvas()->restore();
+ static constexpr int kPad = 2;
+ canvas->translate(kPad, kPad);
+ skiagm::DrawResult result;
+ auto downW = static_cast<int>(kInner / 2);
+ auto downH = static_cast<int>(kInner / 2);
+ result = do_rescale_grid(canvas, surface.get(), srcRect, downW, downH, errorMsg, kPad);
+ if (result != skiagm::DrawResult::kOk) {
+ return result;
+ }
+ canvas->translate(0, 2 * downH);
+ auto upW = static_cast<int>(kInner * 3.5);
+ auto upH = static_cast<int>(kInner * 4.6);
+ result = do_rescale_grid(canvas, surface.get(), srcRect, upW, upH, errorMsg, kPad);
+ if (result != skiagm::DrawResult::kOk) {
+ return result;
+ }
+ return skiagm::DrawResult::kOk;
+}
diff --git a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-ASAN.json b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-ASAN.json
index 2488c9d..886449f 100644
--- a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-ASAN.json
+++ b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-ASAN.json
@@ -484,6 +484,14 @@
"gm",
"_",
"async_rescale_and_read_rose",
+ "pic-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
+ "serialize-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
"tiles_rt-8888",
"gm",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-BonusConfigs.json b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-BonusConfigs.json
index 6122ac5..cd59862 100644
--- a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-BonusConfigs.json
+++ b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-BonusConfigs.json
@@ -560,6 +560,14 @@
"gm",
"_",
"async_rescale_and_read_rose",
+ "pic-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
+ "serialize-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
"tiles_rt-8888",
"gm",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-MSAN.json b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-MSAN.json
index e281f91..2cc77ab 100644
--- a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-MSAN.json
+++ b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Debug-All-MSAN.json
@@ -478,6 +478,14 @@
"gm",
"_",
"async_rescale_and_read_rose",
+ "pic-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
+ "serialize-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
"tiles_rt-8888",
"gm",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-TSAN.json b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-TSAN.json
index db76096..e94617f 100644
--- a/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-TSAN.json
+++ b/infra/bots/recipes/test.expected/Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-TSAN.json
@@ -479,6 +479,14 @@
"gm",
"_",
"async_rescale_and_read_rose",
+ "pic-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
+ "serialize-8888",
+ "gm",
+ "_",
+ "async_rescale_and_read_no_bleed",
"tiles_rt-8888",
"gm",
"_",
diff --git a/infra/bots/recipes/test.py b/infra/bots/recipes/test.py
index d867261..d90e192 100644
--- a/infra/bots/recipes/test.py
+++ b/infra/bots/recipes/test.py
@@ -557,7 +557,8 @@
'async_rescale_and_read_text_down',
'async_rescale_and_read_dog_up',
'async_rescale_and_read_dog_down',
- 'async_rescale_and_read_rose']:
+ 'async_rescale_and_read_rose',
+ 'async_rescale_and_read_no_bleed']:
blacklist([ 'pic-8888', 'gm', '_', test])
blacklist(['serialize-8888', 'gm', '_', test])
diff --git a/src/image/SkSurface.cpp b/src/image/SkSurface.cpp
index 92a300f..a50f581 100644
--- a/src/image/SkSurface.cpp
+++ b/src/image/SkSurface.cpp
@@ -258,7 +258,11 @@
sk_sp<SkSurface> src(SkRef(this));
int srcX = srcRect.fLeft;
int srcY = srcRect.fTop;
+ SkCanvas::SrcRectConstraint constraint = SkCanvas::kStrict_SrcRectConstraint;
+ // Assume we should ignore the rescale linear request if the surface has no color space since
+ // it's unclear how we'd linearize from an unknown color space.
if (rescaleGamma == SkSurface::RescaleGamma::kLinear &&
+ this->getCanvas()->imageInfo().colorSpace() &&
!this->getCanvas()->imageInfo().colorSpace()->gammaIsLinear()) {
auto cs = this->getCanvas()->imageInfo().colorSpace()->makeLinearGamma();
// Promote to F16 color type to preserve precision.
@@ -278,6 +282,7 @@
src = std::move(linearSurf);
srcX = 0;
srcY = 0;
+ constraint = SkCanvas::kFast_SrcRectConstraint;
}
while (stepsX || stepsY) {
int nextW = info.width();
@@ -310,14 +315,14 @@
callback(context, nullptr, 0);
return;
}
- next->getCanvas()->drawImageRect(src->makeImageSnapshot(),
- SkIRect::MakeXYWH(srcX, srcY, srcW, srcH),
- SkRect::MakeWH((float)nextW, (float)nextH), &paint,
- SkCanvas::kFast_SrcRectConstraint);
+ next->getCanvas()->drawImageRect(
+ src->makeImageSnapshot(), SkIRect::MakeXYWH(srcX, srcY, srcW, srcH),
+ SkRect::MakeWH((float)nextW, (float)nextH), &paint, constraint);
src = std::move(next);
srcX = srcY = 0;
srcW = nextW;
srcH = nextH;
+ constraint = SkCanvas::kFast_SrcRectConstraint;
}
static_cast<SkSurface_Base*>(src.get())->onAsyncReadPixels(info, srcX, srcY, callback, context);
}