SkCodec: Always use 0 for filling
This is a behavior change and a simplification.
When an image is incomplete or subset, we fill the remaining/all rows
with the fill color. A virtual method chose the fill color. Here were
the implementations:
- SkGifCodec:
Use transparent. This was changed previously to match Chromium.
- SkPngCodec/SkBmpStandardCodec:
Use the first color in the color table. This made sense when we had to
support kIndex_8, when we had to use an index from the color table.
Using that color for other types ensured that the image looked the same
in e.g. kN32 as kIndex_8. Removing this arbitrary choice simplifies the
code and moves the behavior toward Chromium's.
- SkCodec (default):
Use black for opaque images and transparent for images with alpha. A
theoretical advantage to this decision was that an incomplete image
would look the same in k565 and kN32. I don't think this is a good
enough reason to behave differently from Chromium.
Consolidate them all to just use 0. This results in transparent where
that is something we can specify. For 565 and Gray, this results in
black.
Only change to include headers is the removal of protected methods.
TBR=hcm@google.com
Change-Id: I9a7224b4e91b5c4988f3a87381653e99e40e10a5
Reviewed-on: https://skia-review.googlesource.com/145378
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 18c0a79..3fe7a03 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -284,7 +284,7 @@
// Set the background as transparent. Then, if the RLE code skips pixels,
// the skipped pixels will be transparent.
if (dst) {
- SkSampler::Fill(dstInfo, dst, dstRowBytes, SK_ColorTRANSPARENT, opts.fZeroInitialized);
+ SkSampler::Fill(dstInfo, dst, dstRowBytes, opts.fZeroInitialized);
}
// Adjust the height and the dst if the previous call to decodeRows() left us
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 0ddcbb5..153d081 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -327,12 +327,3 @@
}
}
}
-
-uint64_t SkBmpStandardCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
- const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
- if (colorPtr) {
- return get_color_table_fill_value(dstInfo.colorType(), dstInfo.alphaType(), colorPtr, 0,
- this->colorXform(), false);
- }
- return INHERITED::onGetFillValue(dstInfo);
-}
diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index 60587fb..1790692 100644
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -57,9 +57,6 @@
SkCodec::Result onPrepareToDecode(const SkImageInfo& dstInfo,
const SkCodec::Options& options) override;
-
- uint64_t onGetFillValue(const SkImageInfo&) const override;
-
SkSampler* getSampler(bool createIfNecessary) override {
SkASSERT(fSwizzler);
return fSwizzler.get();
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index d534be0..8bfaae8 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -237,7 +237,7 @@
const size_t bpp = dstInfo.bytesPerPixel();
const size_t offset = prevRect.x() * bpp + prevRect.y() * rowBytes;
void* eraseDst = SkTAddOffset<void>(pixels, offset);
- SkSampler::Fill(info, eraseDst, rowBytes, 0, SkCodec::kNo_ZeroInitialized);
+ SkSampler::Fill(info, eraseDst, rowBytes, SkCodec::kNo_ZeroInitialized);
return true;
}
@@ -575,28 +575,12 @@
}
}
-uint64_t SkCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
- switch (dstInfo.colorType()) {
- case kRGBA_F16_SkColorType: {
- static constexpr uint64_t transparentColor = 0;
- static constexpr uint64_t opaqueColor = ((uint64_t) SK_Half1) << 48;
- return (kOpaque_SkAlphaType == fSrcInfo.alphaType()) ? opaqueColor : transparentColor;
- }
- default: {
- // This not only handles the kN32 case, but also k565, kGray8, since
- // the low bits are zeros.
- return (kOpaque_SkAlphaType == fSrcInfo.alphaType()) ?
- SK_ColorBLACK : SK_ColorTRANSPARENT;
- }
- }
-}
-
static void fill_proc(const SkImageInfo& info, void* dst, size_t rowBytes,
- uint64_t colorOrIndex, SkCodec::ZeroInitialized zeroInit, SkSampler* sampler) {
+ SkCodec::ZeroInitialized zeroInit, SkSampler* sampler) {
if (sampler) {
- sampler->fill(info, dst, rowBytes, colorOrIndex, zeroInit);
+ sampler->fill(info, dst, rowBytes, zeroInit);
} else {
- SkSampler::Fill(info, dst, rowBytes, colorOrIndex, zeroInit);
+ SkSampler::Fill(info, dst, rowBytes, zeroInit);
}
}
@@ -604,7 +588,6 @@
ZeroInitialized zeroInit, int linesRequested, int linesDecoded) {
void* fillDst;
- const uint64_t fillValue = this->getFillValue(info);
const int linesRemaining = linesRequested - linesDecoded;
SkSampler* sampler = this->getSampler(false);
@@ -617,13 +600,13 @@
case kTopDown_SkScanlineOrder: {
const SkImageInfo fillInfo = info.makeWH(fillWidth, linesRemaining);
fillDst = SkTAddOffset<void>(dst, linesDecoded * rowBytes);
- fill_proc(fillInfo, fillDst, rowBytes, fillValue, zeroInit, sampler);
+ fill_proc(fillInfo, fillDst, rowBytes, zeroInit, sampler);
break;
}
case kBottomUp_SkScanlineOrder: {
fillDst = dst;
const SkImageInfo fillInfo = info.makeWH(fillWidth, linesRemaining);
- fill_proc(fillInfo, fillDst, rowBytes, fillValue, zeroInit, sampler);
+ fill_proc(fillInfo, fillDst, rowBytes, zeroInit, sampler);
break;
}
}
diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h
index 65d4575..d7db89f 100644
--- a/src/codec/SkCodecPriv.h
+++ b/src/codec/SkCodecPriv.h
@@ -107,35 +107,6 @@
}
/*
- * Given that the encoded image uses a color table, return the fill value
- */
-static inline uint64_t get_color_table_fill_value(SkColorType dstColorType, SkAlphaType alphaType,
- const SkPMColor* colorPtr, uint8_t fillIndex, SkColorSpaceXform* colorXform, bool isRGBA) {
- SkASSERT(nullptr != colorPtr);
- switch (dstColorType) {
- case kRGBA_8888_SkColorType:
- case kBGRA_8888_SkColorType:
- return colorPtr[fillIndex];
- case kRGB_565_SkColorType:
- return SkPixel32ToPixel16(colorPtr[fillIndex]);
- case kRGBA_F16_SkColorType: {
- SkASSERT(colorXform);
- uint64_t dstColor;
- uint32_t srcColor = colorPtr[fillIndex];
- SkColorSpaceXform::ColorFormat srcFormat =
- isRGBA ? SkColorSpaceXform::kRGBA_8888_ColorFormat
- : SkColorSpaceXform::kBGRA_8888_ColorFormat;
- SkAssertResult(colorXform->apply(select_xform_format(dstColorType), &dstColor,
- srcFormat, &srcColor, 1, alphaType));
- return dstColor;
- }
- default:
- SkASSERT(false);
- return 0;
- }
-}
-
-/*
* Compute row bytes for an image using pixels per byte
*/
static inline size_t compute_row_bytes_ppb(int width, uint32_t pixelsPerByte) {
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index dbff928..036ee0b 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -357,8 +357,7 @@
// fill ignores the width (replaces it with the actual, scaled width).
// But we need to scale in Y.
auto fillInfo = dstInfo.makeWH(0, scaledHeight);
- fSwizzler->fill(fillInfo, fDst, fDstRowBytes, this->getFillValue(dstInfo),
- opts.fZeroInitialized);
+ fSwizzler->fill(fillInfo, fDst, fDstRowBytes, opts.fZeroInitialized);
filledBackground = true;
}
} else {
@@ -397,12 +396,6 @@
return kSuccess;
}
-uint64_t SkGifCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
- // Using transparent as the fill value matches the behavior in Chromium,
- // which ignores the background color.
- return SK_ColorTRANSPARENT;
-}
-
void SkGifCodec::applyXformRow(const SkImageInfo& dstInfo, void* dst, const uint8_t* src) const {
if (this->xformOnDecode()) {
SkASSERT(this->colorXform());
diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 68f9749..21dfd2b 100644
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -47,8 +47,6 @@
bool onRewind() override;
- uint64_t onGetFillValue(const SkImageInfo&) const override;
-
int onGetFrameCount() override;
bool onGetFrameInfo(int, FrameInfo*) const override;
int onGetRepetitionCount() override;
diff --git a/src/codec/SkMaskSwizzler.h b/src/codec/SkMaskSwizzler.h
index 5d2955c..9425174 100644
--- a/src/codec/SkMaskSwizzler.h
+++ b/src/codec/SkMaskSwizzler.h
@@ -39,10 +39,10 @@
/**
* Implement fill using a custom width.
*/
- void fill(const SkImageInfo& info, void* dst, size_t rowBytes, uint64_t colorOrIndex,
- SkCodec::ZeroInitialized zeroInit) override {
+ void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
+ SkCodec::ZeroInitialized zeroInit) override {
const SkImageInfo fillInfo = info.makeWH(fDstWidth, info.height());
- SkSampler::Fill(fillInfo, dst, rowBytes, colorOrIndex, zeroInit);
+ SkSampler::Fill(fillInfo, dst, rowBytes, zeroInit);
}
/**
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 1cb1c74..e9972cc 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -1162,17 +1162,6 @@
return this->decode(rowsDecoded);
}
-uint64_t SkPngCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
- const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
- if (colorPtr) {
- SkAlphaType alphaType = select_xform_alpha(dstInfo.alphaType(),
- this->getInfo().alphaType());
- return get_color_table_fill_value(dstInfo.colorType(), alphaType, colorPtr, 0,
- this->colorXform(), true);
- }
- return INHERITED::onGetFillValue(dstInfo);
-}
-
std::unique_ptr<SkCodec> SkPngCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
Result* result, SkPngChunkReader* chunkReader) {
SkCodec* outCodec = nullptr;
diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 6848cfc..075181c 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -52,7 +52,6 @@
override;
SkEncodedImageFormat onGetEncodedFormat() const override { return SkEncodedImageFormat::kPNG; }
bool onRewind() override;
- uint64_t onGetFillValue(const SkImageInfo&) const override;
SkSampler* getSampler(bool createIfNecessary) override;
void applyXformRow(void* dst, const void* src);
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index cf97b0b..ccede33 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -327,7 +327,6 @@
// We handle filling uninitialized memory here instead of using this->codec().
// this->codec() does not know that we are sampling.
- const uint64_t fillValue = this->codec()->getFillValue(info);
const SkImageInfo fillInfo = info.makeWH(info.width(), 1);
for (; y < nativeSize.height(); y++) {
int srcY = this->codec()->outputScanline(y);
@@ -336,7 +335,7 @@
}
void* rowPtr = SkTAddOffset<void>(pixels, rowBytes * get_dst_coord(srcY, sampleY));
- SkSampler::Fill(fillInfo, rowPtr, rowBytes, fillValue, options.fZeroInitialized);
+ SkSampler::Fill(fillInfo, rowPtr, rowBytes, options.fZeroInitialized);
}
return SkCodec::kIncompleteInput;
}
diff --git a/src/codec/SkSampler.cpp b/src/codec/SkSampler.cpp
index d18410b..b4f9dda 100644
--- a/src/codec/SkSampler.cpp
+++ b/src/codec/SkSampler.cpp
@@ -11,9 +11,13 @@
#include "SkUtils.h"
void SkSampler::Fill(const SkImageInfo& info, void* dst, size_t rowBytes,
- uint64_t colorOrIndex, SkCodec::ZeroInitialized zeroInit) {
+ SkCodec::ZeroInitialized zeroInit) {
SkASSERT(dst != nullptr);
+ if (SkCodec::kYes_ZeroInitialized == zeroInit) {
+ return;
+ }
+
// Calculate bytes to fill.
const size_t bytesToFill = info.computeByteSize(rowBytes);
const int width = info.width();
@@ -23,64 +27,28 @@
switch (info.colorType()) {
case kRGBA_8888_SkColorType:
case kBGRA_8888_SkColorType: {
- // If memory is zero initialized, we may not need to fill
- uint32_t color = (uint32_t) colorOrIndex;
- if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == color) {
- return;
- }
-
uint32_t* dstRow = (uint32_t*) dst;
for (int row = 0; row < numRows; row++) {
- sk_memset32((uint32_t*) dstRow, color, width);
+ sk_memset32((uint32_t*) dstRow, 0, width);
dstRow = SkTAddOffset<uint32_t>(dstRow, rowBytes);
}
break;
}
case kRGB_565_SkColorType: {
- // If the destination is k565, the caller passes in a 16-bit color.
- // We will not assert that the high bits of colorOrIndex must be zeroed.
- // This allows us to take advantage of the fact that the low 16 bits of an
- // SKPMColor may be a valid a 565 color. For example, the low 16
- // bits of SK_ColorBLACK are identical to the 565 representation
- // for black.
-
- // If memory is zero initialized, we may not need to fill
- uint16_t color = (uint16_t) colorOrIndex;
- if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == color) {
- return;
- }
-
uint16_t* dstRow = (uint16_t*) dst;
for (int row = 0; row < numRows; row++) {
- sk_memset16((uint16_t*) dstRow, color, width);
+ sk_memset16((uint16_t*) dstRow, 0, width);
dstRow = SkTAddOffset<uint16_t>(dstRow, rowBytes);
}
break;
}
case kGray_8_SkColorType:
- // If the destination is kGray, the caller passes in an 8-bit color.
- // We will not assert that the high bits of colorOrIndex must be zeroed.
- // This allows us to take advantage of the fact that the low 8 bits of an
- // SKPMColor may be a valid a grayscale color. For example, the low 8
- // bits of SK_ColorBLACK are identical to the grayscale representation
- // for black.
-
- // If memory is zero initialized, we may not need to fill
- if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == (uint8_t) colorOrIndex) {
- return;
- }
-
- memset(dst, (uint8_t) colorOrIndex, bytesToFill);
+ memset(dst, 0, bytesToFill);
break;
case kRGBA_F16_SkColorType: {
- uint64_t color = colorOrIndex;
- if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == color) {
- return;
- }
-
uint64_t* dstRow = (uint64_t*) dst;
for (int row = 0; row < numRows; row++) {
- sk_memset64((uint64_t*) dstRow, color, width);
+ sk_memset64((uint64_t*) dstRow, 0, width);
dstRow = SkTAddOffset<uint64_t>(dstRow, rowBytes);
}
break;
diff --git a/src/codec/SkSampler.h b/src/codec/SkSampler.h
index 8dce671..815153b 100644
--- a/src/codec/SkSampler.h
+++ b/src/codec/SkSampler.h
@@ -45,39 +45,34 @@
}
/**
- * Fill the remainder of the destination with a single color
+ * Fill the remainder of the destination with 0.
+ *
+ * 0 has a different meaning depending on the SkColorType. For color types
+ * with transparency, this means transparent. For k565 and kGray, 0 is
+ * black.
*
* @param info
* Contains the color type of the rows to fill.
- * Contains the width of the destination rows to fill
+ * Contains the pixel width of the destination rows to fill
* Contains the number of rows that we need to fill.
*
* @param dst
- * The destination row to fill from.
+ * The destination row to fill.
*
* @param rowBytes
* Stride in bytes of the destination.
*
- * @param colorOrIndex
- * If colorType is kF16, colorOrIndex is treated as a 64-bit color.
- * If colorType is kN32, colorOrIndex is treated as a 32-bit color.
- * If colorType is k565, colorOrIndex is treated as a 16-bit color.
- * If colorType is kGray, colorOrIndex is treated as an 8-bit color.
- * If colorType is kIndex, colorOrIndex is treated as an 8-bit index.
- * Other SkColorTypes are not supported.
- *
* @param zeroInit
* Indicates whether memory is already zero initialized.
- *
*/
static void Fill(const SkImageInfo& info, void* dst, size_t rowBytes,
- uint64_t colorOrIndex, SkCodec::ZeroInitialized zeroInit);
+ SkCodec::ZeroInitialized zeroInit);
/**
* Allow subclasses to implement unique versions of fill().
*/
virtual void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
- uint64_t colorOrIndex, SkCodec::ZeroInitialized zeroInit) {}
+ SkCodec::ZeroInitialized zeroInit) {} // FIXME: Can this be abstract?
SkSampler()
: fSampleY(1)
diff --git a/src/codec/SkSwizzler.h b/src/codec/SkSwizzler.h
index ebaed7e..df2cf9f 100644
--- a/src/codec/SkSwizzler.h
+++ b/src/codec/SkSwizzler.h
@@ -56,10 +56,10 @@
/**
* Implement fill using a custom width.
*/
- void fill(const SkImageInfo& info, void* dst, size_t rowBytes, uint64_t colorOrIndex,
+ void fill(const SkImageInfo& info, void* dst, size_t rowBytes,
SkCodec::ZeroInitialized zeroInit) override {
const SkImageInfo fillInfo = info.makeWH(fAllocatedWidth, info.height());
- SkSampler::Fill(fillInfo, dst, rowBytes, colorOrIndex, zeroInit);
+ SkSampler::Fill(fillInfo, dst, rowBytes, zeroInit);
}
/**
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index bb6e430..0d12dad 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -442,7 +442,7 @@
SkASSERT(srcInfo.bounds().contains(frameRect));
const bool frameIsSubset = frameRect != srcInfo.bounds();
if (independent && frameIsSubset) {
- SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
+ SkSampler::Fill(dstInfo, dst, rowBytes, options.fZeroInitialized);
}
int dstX = frameRect.x();