Support more swizzles to 565 in SkCodec

Add more swizzling functions for swizzling to 565. Much of this
code was revived from crrev.com/1055743003 (for BMP). Also added
swizzling functions for WBMP.

Consolidate the static function conversion_possible.

In SkCodec::getPixels, check that the alphatype corresponds to the
colorType. This prevents requesting 565 + non-opaque.

In SkIcoCodec, report that the image is unpremul (instead of
whatever the largest embedded codec thinks), but modify the
requested info to have the alpha type expected/required by the
embedded codec.

Add tests for decoding to 565.

BUG=skia:3257
BUG=skia:3683

Review URL: https://codereview.chromium.org/1277213002
This commit is contained in:
scroggo 2015-08-14 08:32:46 -07:00 committed by Commit bot
parent 8f4ba76742
commit cc2feb161f
16 changed files with 250 additions and 154 deletions

View File

@ -13,6 +13,7 @@
#include "ProcStats.h"
#include "SkBBHFactory.h"
#include "SkChecksum.h"
#include "SkCodec.h"
#include "SkCommonFlags.h"
#include "SkFontMgr.h"
#include "SkForceLinking.h"

View File

@ -13,7 +13,6 @@
#include "SkBBoxHierarchy.h"
#include "SkBitmap.h"
#include "SkCanvas.h"
#include "SkCodec.h"
#include "SkData.h"
#include "SkGPipe.h"
#include "SkPicture.h"

View File

@ -397,6 +397,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
iBuffer.free();
// Additionally, 32 bit bmp-in-icos use the alpha channel.
// FIXME (msarett): Don't all bmp-in-icos use the alpha channel?
// And, RLE inputs may skip pixels, leaving them as transparent. This
// is uncommon, but we cannot be certain that an RLE bmp will be opaque.
if ((inIco && 32 == bitsPerPixel) || (kRLE_BmpInputFormat == inputFormat)) {

View File

@ -9,33 +9,6 @@
#include "SkCodecPriv.h"
#include "SkColorPriv.h"
/*
* Checks if the conversion between the input image and the requested output
* image has been implemented
*/
static bool conversion_possible(const SkImageInfo& dst,
const SkImageInfo& src) {
// Ensure that the profile type is unchanged
if (dst.profileType() != src.profileType()) {
return false;
}
// Ensure the alpha type is valid
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
// Check for supported color types
switch (dst.colorType()) {
// Allow output to kN32
case kN32_SkColorType:
return true;
default:
return false;
}
}
/*
* Creates an instance of the decoder
*/

View File

@ -11,35 +11,6 @@
#include "SkScanlineDecoder.h"
#include "SkStream.h"
/*
* Checks if the conversion between the input image and the requested output
* image has been implemented
*/
static bool conversion_possible(const SkImageInfo& dst,
const SkImageInfo& src) {
// Ensure that the profile type is unchanged
if (dst.profileType() != src.profileType()) {
return false;
}
// Ensure the alpha type is valid
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
// Check for supported color types
switch (dst.colorType()) {
// Allow output to kN32 from any type of input
case kN32_SkColorType:
return true;
// Allow output to kIndex_8 from compatible inputs
case kIndex_8_SkColorType:
return kIndex_8_SkColorType == src.colorType();
default:
return false;
}
}
/*
* Creates an instance of the decoder
* Called only by NewFromStream
@ -240,6 +211,11 @@ void SkBmpRLECodec::setPixel(void* dst, size_t dstRowBytes,
dstRow[x] = fColorTable->operator[](index);
break;
}
case kRGB_565_SkColorType: {
uint16_t* dstRow = SkTAddOffset<uint16_t>(dst, row * (int) dstRowBytes);
dstRow[x] = SkPixel32ToPixel16(fColorTable->operator[](index));
break;
}
default:
// This case should not be reached. We should catch an invalid
// color type when we check that the conversion is possible.
@ -272,6 +248,11 @@ void SkBmpRLECodec::setRGBPixel(void* dst, size_t dstRowBytes,
dstRow[x] = SkPackARGB32NoCheck(0xFF, red, green, blue);
break;
}
case kRGB_565_SkColorType: {
uint16_t* dstRow = SkTAddOffset<uint16_t>(dst, row * (int) dstRowBytes);
dstRow[x] = SkPack888ToRGB16(red, green, blue);
break;
}
default:
// This case should not be reached. We should catch an invalid
// color type when we check that the conversion is possible.

View File

@ -11,35 +11,6 @@
#include "SkScanlineDecoder.h"
#include "SkStream.h"
/*
* Checks if the conversion between the input image and the requested output
* image has been implemented
*/
static bool conversion_possible(const SkImageInfo& dst,
const SkImageInfo& src) {
// Ensure that the profile type is unchanged
if (dst.profileType() != src.profileType()) {
return false;
}
// Ensure the alpha type is valid
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
// Check for supported color types
switch (dst.colorType()) {
// Allow output to kN32 from any type of input
case kN32_SkColorType:
return true;
// Allow output to kIndex_8 from compatible inputs
case kIndex_8_SkColorType:
return kIndex_8_SkColorType == src.colorType();
default:
return false;
}
}
/*
* Creates an instance of the decoder
* Called only by NewFromStream
@ -323,6 +294,11 @@ SkCodec::Result SkBmpStandardCodec::decode(const SkImageInfo& dstInfo,
// Finally, apply the AND mask for bmp-in-ico images
if (fInIco) {
// BMP in ICO have transparency, so this cannot be 565, and this mask
// prevents us from using kIndex8. The below code depends on the output
// being an SkPMColor.
SkASSERT(dstInfo.colorType() == kN32_SkColorType);
// The AND mask is always 1 bit per pixel
const size_t rowBytes = SkAlign4(compute_row_bytes(width, 1));

View File

@ -123,6 +123,15 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t
ctable = NULL;
}
{
SkAlphaType canonical;
if (!SkColorTypeValidateAlphaType(info.colorType(), info.alphaType(), &canonical)
|| canonical != info.alphaType())
{
return kInvalidConversion;
}
}
// Default options.
Options optsStorage;
if (NULL == options) {

View File

@ -52,6 +52,35 @@ static inline bool valid_alpha(SkAlphaType dstAlpha, SkAlphaType srcAlpha) {
return true;
}
/*
* Most of our codecs support the same conversions:
* - profileType must be the same
* - opaque only to opaque (and 565 only if opaque)
* - premul to unpremul and vice versa
* - always support N32
* - otherwise match the src color type
*/
static bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) {
if (dst.profileType() != src.profileType()) {
return false;
}
// Ensure the alpha type is valid
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
// Check for supported color types
switch (dst.colorType()) {
case kN32_SkColorType:
return true;
case kRGB_565_SkColorType:
return src.alphaType() == kOpaque_SkAlphaType;
default:
return dst.colorType() == src.colorType();
}
}
/*
* If there is a color table, get a pointer to the colors, otherwise return NULL
*/

View File

@ -210,30 +210,6 @@ SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream,
, fGif(gif)
{}
/*
* Checks if the conversion between the input image and the requested output
* image has been implemented
*/
static bool conversion_possible(const SkImageInfo& dst,
const SkImageInfo& src) {
// Ensure that the profile type is unchanged
if (dst.profileType() != src.profileType()) {
return false;
}
// Check for supported color and alpha types
switch (dst.colorType()) {
case kN32_SkColorType:
return kPremul_SkAlphaType == dst.alphaType() ||
kUnpremul_SkAlphaType == dst.alphaType();
case kIndex_8_SkColorType:
return kPremul_SkAlphaType == dst.alphaType() ||
kUnpremul_SkAlphaType == dst.alphaType();
default:
return false;
}
}
bool SkGifCodec::onRewind() {
GifFileType* gifOut = NULL;
if (!ReadHeader(this->stream(), NULL, &gifOut)) {

View File

@ -176,6 +176,17 @@ SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
}
SkImageInfo info = codecs->operator[](maxIndex)->getInfo();
// ICOs contain an alpha mask after the image which means we cannot
// guarantee that an image is opaque, even if the sub-codec thinks it
// is.
// FIXME (msarett): The BMP decoder depends on the alpha type in order
// to decode correctly, otherwise it could report kUnpremul and we would
// not have to correct it here. Is there a better way?
// FIXME (msarett): This is only true for BMP in ICO - could a PNG in ICO
// be opaque? Is it okay that we missed out on the opportunity to mark
// such an image as opaque?
info = info.makeAlphaType(kUnpremul_SkAlphaType);
// Note that stream is owned by the embedded codec, the ico does not need
// direct access to the stream.
return SkNEW_ARGS(SkIcoCodec, (info, codecs.detach()));
@ -233,17 +244,24 @@ SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
// Subsets are not supported.
return kUnimplemented;
}
if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
return kInvalidConversion;
}
// We return invalid scale if there is no candidate image with matching
// dimensions.
Result result = kInvalidScale;
for (int32_t i = 0; i < fEmbeddedCodecs->count(); i++) {
SkCodec* embeddedCodec = fEmbeddedCodecs->operator[](i);
// If the dimensions match, try to decode
if (dstInfo.dimensions() ==
fEmbeddedCodecs->operator[](i)->getInfo().dimensions()) {
if (dstInfo.dimensions() == embeddedCodec->getInfo().dimensions()) {
// Perform the decode
result = fEmbeddedCodecs->operator[](i)->getPixels(dstInfo,
dst, dstRowBytes, &opts, ct, ptr);
// FIXME: (msarett): ICO is considered non-opaque, even if the embedded BMP
// incorrectly claims it has no alpha.
SkImageInfo info = dstInfo.makeAlphaType(embeddedCodec->getInfo().alphaType());
result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, ct, ptr);
// On a fatal error, keep trying to find an image to decode
if (kInvalidConversion == result || kInvalidInput == result ||

View File

@ -392,28 +392,6 @@ void SkPngCodec::destroyReadStruct() {
// Getting the pixels
///////////////////////////////////////////////////////////////////////////////
static bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) {
// TODO: Support other conversions
if (dst.profileType() != src.profileType()) {
return false;
}
// Ensure the alpha type is valid
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
// Check for supported color types
switch (dst.colorType()) {
case kN32_SkColorType:
return true;
case kRGB_565_SkColorType:
return src.alphaType() == kOpaque_SkAlphaType;
default:
return dst.colorType() == src.colorType();
}
}
SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
const Options& options,
SkPMColor ctable[],

View File

@ -73,13 +73,11 @@ bool SkWbmpCodec::onRewind() {
SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info,
const SkPMColor* ctable, const Options& opts) {
// TODO (msarett): Reenable support for 565 if it is desired
// skbug.com/3683
// Create the swizzler based on the desired color type
switch (info.colorType()) {
case kIndex_8_SkColorType:
case kN32_SkColorType:
case kRGB_565_SkColorType:
case kGray_8_SkColorType:
return SkSwizzler::CreateSwizzler(SkSwizzler::kBit, ctable, info, opts.fZeroInitialized,
this->getInfo());

View File

@ -63,6 +63,24 @@ static SkSwizzler::ResultAlpha swizzle_mask16_to_n32_premul(
return COMPUTE_RESULT_ALPHA;
}
// TODO (msarett): We have promoted a two byte per pixel image to 8888, only to
// convert it back to 565. Instead, we should swizzle to 565 directly.
static SkSwizzler::ResultAlpha swizzle_mask16_to_565(
void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) {
// Use the masks to decode to the destination
uint16_t* srcPtr = (uint16_t*) srcRow;
uint16_t* dstPtr = (uint16_t*) dstRow;
for (int i = 0; i < width; i++) {
uint16_t p = srcPtr[i];
uint8_t red = masks->getRed(p);
uint8_t green = masks->getGreen(p);
uint8_t blue = masks->getBlue(p);
dstPtr[i] = SkPack888ToRGB16(red, green, blue);
}
return SkSwizzler::kOpaque_ResultAlpha;
}
static SkSwizzler::ResultAlpha swizzle_mask24_to_n32_opaque(
void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) {
@ -114,6 +132,21 @@ static SkSwizzler::ResultAlpha swizzle_mask24_to_n32_premul(
return COMPUTE_RESULT_ALPHA;
}
static SkSwizzler::ResultAlpha swizzle_mask24_to_565(
void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) {
// Use the masks to decode to the destination
uint16_t* dstPtr = (uint16_t*) dstRow;
for (int i = 0; i < 3*width; i += 3) {
uint32_t p = srcRow[i] | (srcRow[i + 1] << 8) | srcRow[i + 2] << 16;
uint8_t red = masks->getRed(p);
uint8_t green = masks->getGreen(p);
uint8_t blue = masks->getBlue(p);
dstPtr[i/3] = SkPack888ToRGB16(red, green, blue);
}
return SkSwizzler::kOpaque_ResultAlpha;
}
static SkSwizzler::ResultAlpha swizzle_mask32_to_n32_opaque(
void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) {
@ -168,6 +201,21 @@ static SkSwizzler::ResultAlpha swizzle_mask32_to_n32_premul(
return COMPUTE_RESULT_ALPHA;
}
static SkSwizzler::ResultAlpha swizzle_mask32_to_565(
void* dstRow, const uint8_t* srcRow, int width, SkMasks* masks) {
// Use the masks to decode to the destination
uint32_t* srcPtr = (uint32_t*) srcRow;
uint16_t* dstPtr = (uint16_t*) dstRow;
for (int i = 0; i < width; i++) {
uint32_t p = srcPtr[i];
uint8_t red = masks->getRed(p);
uint8_t green = masks->getGreen(p);
uint8_t blue = masks->getBlue(p);
dstPtr[i] = SkPack888ToRGB16(red, green, blue);
}
return SkSwizzler::kOpaque_ResultAlpha;
}
/*
*
* Create a new mask swizzler
@ -196,6 +244,15 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(
break;
}
break;
case kRGB_565_SkColorType:
switch (info.alphaType()) {
case kOpaque_SkAlphaType:
proc = &swizzle_mask16_to_565;
break;
default:
break;
}
break;
default:
break;
}
@ -217,6 +274,15 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(
break;
}
break;
case kRGB_565_SkColorType:
switch (info.alphaType()) {
case kOpaque_SkAlphaType:
proc = &swizzle_mask24_to_565;
break;
default:
break;
}
break;
default:
break;
}
@ -238,6 +304,15 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(
break;
}
break;
case kRGB_565_SkColorType:
switch (info.alphaType()) {
case kOpaque_SkAlphaType:
proc = &swizzle_mask32_to_565;
break;
default:
break;
}
break;
default:
break;
}

View File

@ -114,6 +114,34 @@ static SkSwizzler::ResultAlpha swizzle_bit_to_n32(
return SkSwizzler::kOpaque_ResultAlpha;
}
#define RGB565_BLACK 0
#define RGB565_WHITE 0xFFFF
static SkSwizzler::ResultAlpha swizzle_bit_to_565(
void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
// increment src by byte offset and bitIndex by bit offset
src += offset / 8;
int bitIndex = offset % 8;
uint8_t currByte = *src;
dst[0] = ((currByte >> (7 - bitIndex)) & 1) ? RGB565_WHITE : RGB565_BLACK;
for (int x = 1; x < dstWidth; x++) {
int bitOffset = bitIndex + deltaSrc;
bitIndex = bitOffset % 8;
currByte = *(src += bitOffset / 8);
dst[x] = ((currByte >> (7 - bitIndex)) & 1) ? RGB565_WHITE : RGB565_BLACK;
}
return SkSwizzler::kOpaque_ResultAlpha;
}
#undef RGB565_BLACK
#undef RGB565_WHITE
// kIndex1, kIndex2, kIndex4
static SkSwizzler::ResultAlpha swizzle_small_index_to_index(
@ -140,6 +168,29 @@ static SkSwizzler::ResultAlpha swizzle_small_index_to_index(
return COMPUTE_RESULT_ALPHA;
}
static SkSwizzler::ResultAlpha swizzle_small_index_to_565(
void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
int bitsPerPixel, int offset, const SkPMColor ctable[]) {
src += offset;
uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
const uint32_t pixelsPerByte = 8 / bitsPerPixel;
const size_t rowBytes = compute_row_bytes_ppb(dstWidth, pixelsPerByte);
const uint8_t mask = (1 << bitsPerPixel) - 1;
int x = 0;
for (uint32_t byte = 0; byte < rowBytes; byte++) {
uint8_t pixelData = src[byte];
for (uint32_t p = 0; p < pixelsPerByte && x < dstWidth; p++) {
uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
uint16_t c = SkPixel32ToPixel16(ctable[index]);
dst[x] = c;
pixelData <<= bitsPerPixel;
x++;
}
}
return SkSwizzler::kOpaque_ResultAlpha;
}
static SkSwizzler::ResultAlpha swizzle_small_index_to_n32(
void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
int bitsPerPixel, int offset, const SkPMColor ctable[]) {
@ -306,6 +357,19 @@ static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32(
return SkSwizzler::kOpaque_ResultAlpha;
}
static SkSwizzler::ResultAlpha swizzle_bgrx_to_565(
void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
int deltaSrc, int offset, const SkPMColor ctable[]) {
// FIXME: Support dithering?
src += offset;
uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
for (int x = 0; x < dstWidth; x++) {
dst[x] = SkPack888ToRGB16(src[2], src[1], src[0]);
src += deltaSrc;
}
return SkSwizzler::kOpaque_ResultAlpha;
}
// kBGRA
static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_unpremul(
@ -470,6 +534,9 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc,
case kIndex_8_SkColorType:
proc = &swizzle_bit_to_index;
break;
case kRGB_565_SkColorType:
proc = &swizzle_bit_to_565;
break;
case kGray_8_SkColorType:
proc = &swizzle_bit_to_grayscale;
break;
@ -484,6 +551,9 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc,
case kN32_SkColorType:
proc = &swizzle_small_index_to_n32;
break;
case kRGB_565_SkColorType:
proc = &swizzle_small_index_to_565;
break;
case kIndex_8_SkColorType:
proc = &swizzle_small_index_to_index;
break;
@ -534,6 +604,9 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc,
case kN32_SkColorType:
proc = &swizzle_bgrx_to_n32;
break;
case kRGB_565_SkColorType:
proc = &swizzle_bgrx_to_565;
break;
default:
break;
}

View File

@ -5,6 +5,7 @@
* found in the LICENSE file.
*/
#include "SkCodecPriv.h"
#include "SkWebpCodec.h"
#include "SkTemplates.h"
@ -81,29 +82,27 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
return NULL;
}
static bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) {
// This version is slightly different from SkCodecPriv's version of conversion_possible. It
// supports both byte orders for 8888.
static bool webp_conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) {
if (dst.profileType() != src.profileType()) {
return false;
}
if (!valid_alpha(dst.alphaType(), src.alphaType())) {
return false;
}
switch (dst.colorType()) {
// Both byte orders are supported.
case kBGRA_8888_SkColorType:
case kRGBA_8888_SkColorType:
break;
return true;
case kRGB_565_SkColorType:
if (src.alphaType() == kOpaque_SkAlphaType
&& dst.alphaType() == kOpaque_SkAlphaType)
{
return true;
}
return src.alphaType() == kOpaque_SkAlphaType;
default:
return false;
}
if (dst.alphaType() == src.alphaType()) {
return true;
}
return kPremul_SkAlphaType == dst.alphaType() &&
kUnpremul_SkAlphaType == src.alphaType();
}
SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const {
@ -157,7 +156,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
return kCouldNotRewind;
}
if (!conversion_possible(dstInfo, this->getInfo())) {
if (!webp_conversion_possible(dstInfo, this->getInfo())) {
return kInvalidConversion;
}

View File

@ -78,7 +78,8 @@ static void check(skiatest::Reporter* r,
const char path[],
SkISize size,
bool supportsScanlineDecoding,
bool supportsSubsetDecoding) {
bool supportsSubsetDecoding,
bool supports565 = true) {
SkAutoTDelete<SkStream> stream(resource(path));
if (!stream) {
SkDebugf("Missing resource '%s'\n", path);
@ -96,6 +97,15 @@ static void check(skiatest::Reporter* r,
// decodes to all possible destination color types.
SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
REPORTER_ASSERT(r, info.dimensions() == size);
{
// Test decoding to 565
SkImageInfo info565 = info.makeColorType(kRGB_565_SkColorType);
SkCodec::Result expected = (supports565 && info.alphaType() == kOpaque_SkAlphaType) ?
SkCodec::kSuccess : SkCodec::kInvalidConversion;
test_info(r, codec, info565, expected, NULL);
}
SkBitmap bm;
bm.allocPixels(info);
SkAutoLockPixels autoLockPixels(bm);
@ -213,7 +223,7 @@ DEF_TEST(Codec, r) {
check(r, "randPixels.gif", SkISize::Make(8, 8), false, false);
// JPG
check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false);
check(r, "CMYK.jpg", SkISize::Make(642, 516), true, false, false);
check(r, "color_wheel.jpg", SkISize::Make(128, 128), true, false);
check(r, "grayscale.jpg", SkISize::Make(128, 128), true, false);
check(r, "mandrill_512_q075.jpg", SkISize::Make(512, 512), true, false);