Revert of add colortable support to imagegenerator (https://codereview.chromium.org/304443003/)
Reason for revert:
failing tests
Original issue's description:
> add colortable support to imagegenerator
>
> BUG=skia:
>
> Committed: http://code.google.com/p/skia/source/detail?r=14916
R=halcanary@google.com, scroggo@google.com
TBR=halcanary@google.com, scroggo@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Author: reed@google.com
Review URL: https://codereview.chromium.org/300873007
git-svn-id: http://skia.googlecode.com/svn/trunk@14917 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gyp/images.gyp b/gyp/images.gyp
index 18a18be..0062f24 100644
--- a/gyp/images.gyp
+++ b/gyp/images.gyp
@@ -71,7 +71,6 @@
'../src/images/SkImageEncoder.cpp',
'../src/images/SkImageEncoder_Factory.cpp',
'../src/images/SkImageEncoder_argb.cpp',
- '../src/images/SkImageGenerator.cpp',
'../src/images/SkImageRef.cpp',
'../src/images/SkImageRefPool.cpp',
'../src/images/SkImageRefPool.h',
diff --git a/gyp/skia_for_chromium_defines.gypi b/gyp/skia_for_chromium_defines.gypi
index a47b391..c114b71 100644
--- a/gyp/skia_for_chromium_defines.gypi
+++ b/gyp/skia_for_chromium_defines.gypi
@@ -16,7 +16,6 @@
'SK_SUPPORT_LEGACY_GETTOPDEVICE',
'SK_SUPPORT_LEGACY_ASIMAGEINFO',
'SK_SUPPORT_LEGACY_N32_NAME',
- 'SK_SUPPORT_LEGACY_IMAGEGENERATORAPI',
],
},
}
diff --git a/include/core/SkImageGenerator.h b/include/core/SkImageGenerator.h
index 21ebcbb..f399ab5 100644
--- a/include/core/SkImageGenerator.h
+++ b/include/core/SkImageGenerator.h
@@ -9,7 +9,6 @@
#define SkImageGenerator_DEFINED
#include "SkImageInfo.h"
-#include "SkColor.h"
class SkBitmap;
class SkData;
@@ -48,13 +47,6 @@
*/
virtual ~SkImageGenerator() { }
-#ifdef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI
- virtual bool refEncodedData() { return this->onRefEncodedData(); }
- virtual bool getInfo(SkImageInfo* info) { return this->onGetInfo(info); }
- virtual bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
- return this->onGetPixels(info, pixels, rowBytes, NULL, NULL);
- }
-#else
/**
* Return a ref to the encoded (i.e. compressed) representation,
* of this data.
@@ -62,7 +54,7 @@
* If non-NULL is returned, the caller is responsible for calling
* unref() on the data when it is finished.
*/
- SkData* refEncodedData() { return this->onRefEncodedData(); }
+ virtual SkData* refEncodedData() { return NULL; }
/**
* Return some information about the image, allowing the owner of
@@ -73,7 +65,7 @@
*
* @return false if anything goes wrong.
*/
- bool getInfo(SkImageInfo* info);
+ virtual bool getInfo(SkImageInfo* info) = 0;
/**
* Decode into the given pixels, a block of memory of size at
@@ -91,31 +83,12 @@
* different output-configs, which the implementation can
* decide to support or not.
*
- * If info is kIndex8_SkColorType, then the caller must provide storage for up to 256
- * SkPMColor values in ctable. On success the generator must copy N colors into that storage,
- * (where N is the logical number of table entries) and set ctableCount to N.
- *
- * If info is not kIndex8_SkColorType, then the last two parameters may be NULL. If ctableCount
- * is not null, it will be set to 0.
- *
* @return false if anything goes wrong or if the image info is
* unsupported.
*/
- bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
- SkPMColor ctable[], int* ctableCount);
-
- /**
- * Simplified version of getPixels() that asserts that info is NOT kIndex8_SkColorType.
- */
- bool getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes);
-#endif
-
-protected:
- virtual SkData* onRefEncodedData();
- virtual bool onGetInfo(SkImageInfo* info);
- virtual bool onGetPixels(const SkImageInfo& info,
- void* pixels, size_t rowBytes,
- SkPMColor ctable[], int* ctableCount);
+ virtual bool getPixels(const SkImageInfo& info,
+ void* pixels,
+ size_t rowBytes) = 0;
};
#endif // SkImageGenerator_DEFINED
diff --git a/src/images/SkDecodingImageGenerator.cpp b/src/images/SkDecodingImageGenerator.cpp
index 1f0f706..b3924a7 100644
--- a/src/images/SkDecodingImageGenerator.cpp
+++ b/src/images/SkDecodingImageGenerator.cpp
@@ -23,6 +23,12 @@
class DecodingImageGenerator : public SkImageGenerator {
public:
virtual ~DecodingImageGenerator();
+ virtual SkData* refEncodedData() SK_OVERRIDE;
+ // This implementaion of getInfo() always returns true.
+ virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE;
+ virtual bool getPixels(const SkImageInfo& info,
+ void* pixels,
+ size_t rowBytes) SK_OVERRIDE;
SkData* fData;
SkStreamRewindable* fStream;
@@ -35,18 +41,6 @@
const SkImageInfo& info,
int sampleSize,
bool ditherImage);
-
-protected:
- virtual SkData* onRefEncodedData() SK_OVERRIDE;
- virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
- *info = fInfo;
- return true;
- }
- virtual bool onGetPixels(const SkImageInfo& info,
- void* pixels, size_t rowBytes,
- SkPMColor ctable[], int* ctableCount) SK_OVERRIDE;
-
-private:
typedef SkImageGenerator INHERITED;
};
@@ -129,7 +123,14 @@
fStream->unref();
}
-SkData* DecodingImageGenerator::onRefEncodedData() {
+bool DecodingImageGenerator::getInfo(SkImageInfo* info) {
+ if (info != NULL) {
+ *info = fInfo;
+ }
+ return true;
+}
+
+SkData* DecodingImageGenerator::refEncodedData() {
// This functionality is used in `gm --serialize`
// Does not encode options.
if (fData != NULL) {
@@ -150,15 +151,22 @@
return SkSafeRef(fData);
}
-bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info,
- void* pixels, size_t rowBytes,
- SkPMColor ctableEntries[], int* ctableCount) {
+bool DecodingImageGenerator::getPixels(const SkImageInfo& info,
+ void* pixels,
+ size_t rowBytes) {
+ if (NULL == pixels) {
+ return false;
+ }
if (fInfo != info) {
// The caller has specified a different info. This is an
// error for this kind of SkImageGenerator. Use the Options
// to change the settings.
return false;
}
+ if (info.minRowBytes() > rowBytes) {
+ // The caller has specified a bad rowBytes.
+ return false;
+ }
SkAssertResult(fStream->rewind());
SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream));
@@ -194,20 +202,6 @@
} else {
SkASSERT(check_alpha(info.alphaType(), bitmap.alphaType()));
}
-
- if (kIndex_8_SkColorType == info.colorType()) {
- if (kIndex_8_SkColorType != bitmap.colorType()) {
- return false; // they asked for Index8, but we didn't receive that from decoder
- }
- SkColorTable* ctable = bitmap.getColorTable();
- if (NULL == ctable) {
- return false;
- }
- const int count = ctable->count();
- memcpy(ctableEntries, ctable->lockColors(), count * sizeof(SkPMColor));
- ctable->unlockColors();
- *ctableCount = count;
- }
return true;
}
@@ -220,6 +214,11 @@
const SkDecodingImageGenerator::Options& opts) {
SkASSERT(stream);
SkAutoTUnref<SkStreamRewindable> autoStream(stream); // always unref this.
+ if (opts.fUseRequestedColorType &&
+ (kIndex_8_SkColorType == opts.fRequestedColorType)) {
+ // We do not support indexed color with SkImageGenerators,
+ return NULL;
+ }
SkAssertResult(autoStream->rewind());
SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(autoStream));
if (NULL == decoder.get()) {
@@ -228,16 +227,24 @@
SkBitmap bitmap;
decoder->setSampleSize(opts.fSampleSize);
decoder->setRequireUnpremultipliedColors(opts.fRequireUnpremul);
- if (!decoder->decode(stream, &bitmap, SkImageDecoder::kDecodeBounds_Mode)) {
+ if (!decoder->decode(stream, &bitmap,
+ SkImageDecoder::kDecodeBounds_Mode)) {
return NULL;
}
- if (kUnknown_SkColorType == bitmap.colorType()) {
+ if (bitmap.config() == SkBitmap::kNo_Config) {
return NULL;
}
SkImageInfo info = bitmap.info();
- if (opts.fUseRequestedColorType && (opts.fRequestedColorType != info.colorType())) {
+ if (!opts.fUseRequestedColorType) {
+ // Use default
+ if (kIndex_8_SkColorType == bitmap.colorType()) {
+ // We don't support kIndex8 because we don't support
+ // colortables in this workflow.
+ info.fColorType = kN32_SkColorType;
+ }
+ } else {
if (!bitmap.canCopyTo(opts.fRequestedColorType)) {
SkASSERT(bitmap.colorType() != opts.fRequestedColorType);
return NULL; // Can not translate to needed config.
diff --git a/src/images/SkImageGenerator.cpp b/src/images/SkImageGenerator.cpp
deleted file mode 100644
index b898764..0000000
--- a/src/images/SkImageGenerator.cpp
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * Copyright 2014 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkImageGenerator.h"
-
-#ifndef SK_SUPPORT_LEGACY_IMAGEGENERATORAPI
-bool SkImageGenerator::getInfo(SkImageInfo* info) {
- SkImageInfo dummy;
- if (NULL == info) {
- info = &dummy;
- }
- return this->onGetInfo(info);
-}
-
-bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
- SkPMColor ctable[], int* ctableCount) {
- // Dummy (out-of-range) value to catch subclasses that forgot to respect ctable params.
- int localCTableCount = -1;
- int* localCTableCountPtr = NULL;
-
- if (kIndex_8_SkColorType == info.colorType()) {
- if (NULL == ctable || NULL == ctableCount) {
- return false;
- }
- } else {
- if (ctableCount) {
- *ctableCount = 0;
- }
- ctableCount = NULL;
- ctable = NULL;
- }
-
- if (kUnknown_SkColorType == info.colorType()) {
- return false;
- }
- if (NULL == pixels) {
- return false;
- }
- if (rowBytes < info.minRowBytes()) {
- return false;
- }
-
- bool success = this->onGetPixels(info, pixels, rowBytes, ctable, localCTableCountPtr);
-
- if (success && localCTableCountPtr) {
- SkASSERT(localCTableCount >= 0 && localCTableCount <= 256);
- if (ctableCount) {
- *ctableCount = localCTableCount;
- }
- }
- return success;
-}
-
-bool SkImageGenerator::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
- SkASSERT(kIndex_8_SkColorType != info.colorType());
- if (kIndex_8_SkColorType == info.colorType()) {
- return false;
- }
- return this->getPixels(info, pixels, rowBytes, NULL, NULL);
-}
-#endif
-
-/////////////////////////////////////////////////////////////////////////////////////////////
-
-SkData* SkImageGenerator::onRefEncodedData() {
- return NULL;
-}
-
-bool SkImageGenerator::onGetInfo(SkImageInfo*) {
- return false;
-}
-
-bool SkImageGenerator::onGetPixels(const SkImageInfo&, void*, size_t, SkPMColor*, int*) {
- return false;
-}
diff --git a/src/lazy/SkDiscardablePixelRef.cpp b/src/lazy/SkDiscardablePixelRef.cpp
index b0bbd27..ccf812c 100644
--- a/src/lazy/SkDiscardablePixelRef.cpp
+++ b/src/lazy/SkDiscardablePixelRef.cpp
@@ -60,30 +60,15 @@
}
void* pixels = fDiscardableMemory->data();
- const SkImageInfo& info = this->info();
- SkPMColor colors[256];
- int colorCount = 0;
-
- if (!fGenerator->getPixels(info, pixels, fRowBytes, colors, &colorCount)) {
+ if (!fGenerator->getPixels(this->info(), pixels, fRowBytes)) {
fDiscardableMemory->unlock();
SkDELETE(fDiscardableMemory);
fDiscardableMemory = NULL;
return false;
}
- // Note: our ctable is not purgable, as it is not stored in the discardablememory block.
- // This is because SkColorTable is refcntable, and therefore our caller could hold onto it
- // beyond the scope of a lock/unlock. If we change the API/lifecycle for SkColorTable, we
- // could move it into the block, but then again perhaps it is small enough that this doesn't
- // really matter.
- if (colorCount > 0) {
- fCTable.reset(SkNEW_ARGS(SkColorTable, (colors, colorCount)));
- } else {
- fCTable.reset(NULL);
- }
-
rec->fPixels = pixels;
- rec->fColorTable = fCTable.get();
+ rec->fColorTable = NULL;
rec->fRowBytes = fRowBytes;
return true;
}
diff --git a/src/lazy/SkDiscardablePixelRef.h b/src/lazy/SkDiscardablePixelRef.h
index 52a1d6c..e5e1c9f 100644
--- a/src/lazy/SkDiscardablePixelRef.h
+++ b/src/lazy/SkDiscardablePixelRef.h
@@ -17,6 +17,10 @@
* A PixelRef backed by SkDiscardableMemory, with the ability to
* re-generate the pixels (via a SkImageGenerator) if the DM is
* purged.
+ *
+ * Since SkColorTable is reference-counted, we do not support indexed
+ * color with this class; there would be no way for the discardable
+ * memory system to unref the color table.
*/
class SkDiscardablePixelRef : public SkPixelRef {
public:
@@ -42,7 +46,6 @@
// PixelRef, since the SkBitmap doesn't expect them to change.
SkDiscardableMemory* fDiscardableMemory;
- SkAutoTUnref<SkColorTable> fCTable;
/* Takes ownership of SkImageGenerator. */
SkDiscardablePixelRef(const SkImageInfo&, SkImageGenerator*,
diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp
index eff77e2..10e33ba 100644
--- a/tests/CachedDecodingPixelRefTest.cpp
+++ b/tests/CachedDecodingPixelRefTest.cpp
@@ -183,9 +183,7 @@
SkASSERT((fType <= kLast_TestType) && (fType >= 0));
}
virtual ~TestImageGenerator() { }
-
-protected:
- virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
+ virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE {
REPORTER_ASSERT(fReporter, NULL != info);
if ((NULL == info) || (kFailGetInfo_TestType == fType)) {
return false;
@@ -196,9 +194,9 @@
info->fAlphaType = kOpaque_SkAlphaType;
return true;
}
-
- virtual bool onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
- SkPMColor ctable[], int* ctableCount) SK_OVERRIDE {
+ virtual bool getPixels(const SkImageInfo& info,
+ void* pixels,
+ size_t rowBytes) SK_OVERRIDE {
REPORTER_ASSERT(fReporter, pixels != NULL);
size_t minRowBytes
= static_cast<size_t>(info.fWidth * info.bytesPerPixel());
diff --git a/tests/DrawBitmapRectTest.cpp b/tests/DrawBitmapRectTest.cpp
index aa69de4..f294ae7 100644
--- a/tests/DrawBitmapRectTest.cpp
+++ b/tests/DrawBitmapRectTest.cpp
@@ -22,16 +22,20 @@
public:
FailureImageGenerator() { }
virtual ~FailureImageGenerator() { }
-
-protected:
- virtual bool onGetInfo(SkImageInfo* info) SK_OVERRIDE {
+ virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE {
info->fWidth = 100;
info->fHeight = 100;
info->fColorType = kN32_SkColorType;
info->fAlphaType = kPremul_SkAlphaType;
return true;
}
- // default onGetPixels() returns false, which is what we want.
+ virtual bool getPixels(const SkImageInfo& info,
+ void* pixels,
+ size_t rowBytes) SK_OVERRIDE {
+ // this will deliberately return false if they are asking us
+ // to decode into pixels.
+ return false;
+ }
};
// crbug.com/295895