Fix various SkBitmap::copyTo() bugs
I believe that the best fix is to move this entire function into the
framework and call SkPixmap::readPixels() from there. Given that,
we have already branched, I think this is the simplest fix.
(1) We never added F16 support. Enable F16. This involves assigning
color spaces to src and dst pixmaps so that the conversion makes
sense.
(2) SkPixmap::readPixels() does not support parametric transfer
functions. Fortunately, the framework never uses copyTo() to
do color space conversions. We should at least support the
case where the src color space matches the dst color space.
Bug: skia:
Change-Id: I129b7c02249eed34a9ec4aa0ca736aadccc19777
Reviewed-on: https://skia-review.googlesource.com/11387
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 1516505..0e22e30 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -675,6 +675,7 @@
case kRGB_565_SkColorType:
case kRGBA_8888_SkColorType:
case kBGRA_8888_SkColorType:
+ case kRGBA_F16_SkColorType:
break;
case kGray_8_SkColorType:
if (!sameConfigs) {
@@ -735,14 +736,36 @@
return false;
}
SkPixmap srcPM = srcUnlocker.pixmap();
- if (kRGB_565_SkColorType == dstColorType && kOpaque_SkAlphaType != srcPM.alphaType()) {
- // copyTo() is not strict on alpha type. Here we set the src to opaque to allow
- // the call to readPixels() to succeed and preserve this lenient behavior.
- srcPM = SkPixmap(srcPM.info().makeAlphaType(kOpaque_SkAlphaType), srcPM.addr(),
- srcPM.rowBytes(), srcPM.ctable());
+
+ // Various Android specific compatibility modes.
+ // TODO:
+ // Move the logic of this entire function into the framework, then call readPixels() directly.
+ SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType);
+ switch (dstColorType) {
+ case kRGB_565_SkColorType:
+ // copyTo() is not strict on alpha type. Here we set the src to opaque to allow
+ // the call to readPixels() to succeed and preserve this lenient behavior.
+ if (kOpaque_SkAlphaType != srcPM.alphaType()) {
+ srcPM = SkPixmap(srcPM.info().makeAlphaType(kOpaque_SkAlphaType), srcPM.addr(),
+ srcPM.rowBytes(), srcPM.ctable());
+ dstInfo = dstInfo.makeAlphaType(kOpaque_SkAlphaType);
+ }
+ break;
+ case kRGBA_F16_SkColorType:
+ // The caller does not have an opportunity to pass a dst color space. Assume that
+ // they want linear sRGB.
+ dstInfo = dstInfo.makeColorSpace(SkColorSpace::MakeSRGBLinear());
+
+ if (!srcPM.colorSpace()) {
+ // We can't do a sane conversion to F16 without a dst color space. Guess sRGB
+ // in this case.
+ srcPM.setColorSpace(SkColorSpace::MakeSRGB());
+ }
+ break;
+ default:
+ break;
}
- const SkImageInfo dstInfo = srcPM.info().makeColorType(dstColorType);
SkBitmap tmpDst;
if (!tmpDst.setInfo(dstInfo)) {
return false;
@@ -762,7 +785,22 @@
return false;
}
- if (!srcPM.readPixels(dstUnlocker.pixmap())) {
+ SkPixmap dstPM = dstUnlocker.pixmap();
+
+ // We can't do a sane conversion from F16 without a src color space. Guess sRGB in this case.
+ if (kRGBA_F16_SkColorType == srcPM.colorType() && !dstPM.colorSpace()) {
+ dstPM.setColorSpace(SkColorSpace::MakeSRGB());
+ }
+
+ // readPixels does not yet support color spaces with parametric transfer functions. This
+ // works around that restriction when the color spaces are equal.
+ if (kRGBA_F16_SkColorType != dstColorType && kRGBA_F16_SkColorType != srcPM.colorType() &&
+ dstPM.colorSpace() == srcPM.colorSpace()) {
+ dstPM.setColorSpace(nullptr);
+ srcPM.setColorSpace(nullptr);
+ }
+
+ if (!srcPM.readPixels(dstPM)) {
return false;
}