Improvements to readpixels GM
We don't support unpremultiplied images. Therefore:
- Don't test unpremul source images.
- After doing an unpremultiplying read, make sure to premul before drawing
For this to work with F16, add support for F16 sources to
SkColorSpaceXform.
Public API change is comments-only.
TBR=reed@google.com
BUG=skia:
Change-Id: Ie05b58231e99ca88cd7792b65ffbb4f390b01726
Reviewed-on: https://skia-review.googlesource.com/9900
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
diff --git a/gm/readpixels.cpp b/gm/readpixels.cpp
index f481e26..dd84506 100644
--- a/gm/readpixels.cpp
+++ b/gm/readpixels.cpp
@@ -10,6 +10,8 @@
#include "SkCodec.h"
#include "SkColorSpace.h"
#include "SkColorSpace_Base.h"
+#include "SkColorSpaceXform.h"
+#include "SkColorSpaceXformPriv.h"
#include "SkHalf.h"
#include "SkImage.h"
#include "SkPictureRecorder.h"
@@ -48,14 +50,14 @@
static const int kWidth = 64;
static const int kHeight = 64;
-static sk_sp<SkImage> make_raster_image(SkColorType colorType, SkAlphaType alphaType) {
+static sk_sp<SkImage> make_raster_image(SkColorType colorType) {
std::unique_ptr<SkStream> stream(GetResourceAsStream("google_chrome.ico"));
std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
SkBitmap bitmap;
SkImageInfo info = codec->getInfo().makeWH(kWidth, kHeight)
.makeColorType(colorType)
- .makeAlphaType(alphaType)
+ .makeAlphaType(kPremul_SkAlphaType)
.makeColorSpace(fix_for_colortype(codec->getInfo().colorSpace(), colorType));
bitmap.allocPixels(info);
codec->getPixels(info, bitmap.getPixels(), bitmap.rowBytes());
@@ -132,6 +134,17 @@
dstAlphaType, dstColorSpace);
image->readPixels(dstInfo, data->writable_data(), rowBytes, 0, 0, hint);
+ // SkImage must be premul, so manually premul the data if we unpremul'd during readPixels
+ if (kUnpremul_SkAlphaType == dstAlphaType) {
+ auto xform = SkColorSpaceXform::New(dstColorSpace.get(), dstColorSpace.get());
+ if (!xform->apply(select_xform_format(dstColorType), data->writable_data(),
+ select_xform_format(dstColorType), data->data(),
+ image->width() * image->height(), kPremul_SkAlphaType)) {
+ memset(data->writable_data(), 0, rowBytes * image->height());
+ }
+ dstInfo = dstInfo.makeAlphaType(kPremul_SkAlphaType);
+ }
+
// readPixels() does not always clamp F16. The drawing code expects pixels in the 0-1 range.
clamp_if_necessary(dstInfo, data->writable_data());
@@ -152,7 +165,7 @@
}
SkISize onISize() override {
- return SkISize::Make(6 * kWidth, 18 * kHeight);
+ return SkISize::Make(6 * kWidth, 9 * kHeight);
}
void onDraw(SkCanvas* canvas) override {
@@ -178,19 +191,17 @@
for (sk_sp<SkColorSpace> dstColorSpace : colorSpaces) {
for (SkColorType srcColorType : colorTypes) {
- for (SkAlphaType srcAlphaType : alphaTypes) {
- canvas->save();
- sk_sp<SkImage> image = make_raster_image(srcColorType, srcAlphaType);
- for (SkColorType dstColorType : colorTypes) {
- for (SkAlphaType dstAlphaType : alphaTypes) {
- draw_image(canvas, image.get(), dstColorType, dstAlphaType,
- dstColorSpace, SkImage::kAllow_CachingHint);
- canvas->translate((float) kWidth, 0.0f);
- }
+ canvas->save();
+ sk_sp<SkImage> image = make_raster_image(srcColorType);
+ for (SkColorType dstColorType : colorTypes) {
+ for (SkAlphaType dstAlphaType : alphaTypes) {
+ draw_image(canvas, image.get(), dstColorType, dstAlphaType,
+ dstColorSpace, SkImage::kAllow_CachingHint);
+ canvas->translate((float) kWidth, 0.0f);
}
- canvas->restore();
- canvas->translate(0.0f, (float) kHeight);
}
+ canvas->restore();
+ canvas->translate(0.0f, (float) kHeight);
}
}
}
diff --git a/include/core/SkColorSpaceXform.h b/include/core/SkColorSpaceXform.h
index 0c4fd7a..fc38b7c 100644
--- a/include/core/SkColorSpaceXform.h
+++ b/include/core/SkColorSpaceXform.h
@@ -32,15 +32,15 @@
kRGB_U16_BE_ColorFormat, // Src only
kRGBA_U16_BE_ColorFormat, // Src only
- kRGBA_F16_ColorFormat, // Dst only
+ kRGBA_F16_ColorFormat,
kRGBA_F32_ColorFormat, // Dst only
};
/**
* Apply the color conversion to a |src| buffer, storing the output in the |dst| buffer.
*
- * F16 and F32 are only supported as dst color formats, and only when the dst color space
- * is linear. This function will return false in unsupported cases.
+ * F32 is only supported as a dst color format. F16 and F32 are only supported when the color
+ * space is linear. This function will return false in unsupported cases.
*
* @param dst Stored in the format described by |dstColorFormat|
* @param src Stored in the format described by |srcColorFormat|
diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index 067868d..33e01c9 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -1061,6 +1061,7 @@
}
if (kRGBA_F32_ColorFormat == dstColorFormat ||
+ kRGBA_F16_ColorFormat == srcColorFormat ||
kRGBA_U16_BE_ColorFormat == srcColorFormat ||
kRGB_U16_BE_ColorFormat == srcColorFormat ||
kPremul_SkAlphaType == alphaType)
@@ -1163,6 +1164,12 @@
pipeline.append(SkRasterPipeline::swap_rb);
break;
+ case kRGBA_F16_ColorFormat:
+ if (kLinear_SrcGamma != fSrcGamma) {
+ return false;
+ }
+ pipeline.append(SkRasterPipeline::load_f16, &src);
+ break;
case kRGBA_U16_BE_ColorFormat:
switch (fSrcGamma) {
case kLinear_SrcGamma: