Cleanup and simplify some skiaserve and utility code
This started with a search for gammaCloseToSRGB and turned into me
removing a bunch of really crazy bitmap code here. Like DM, just use
SkPngEncoder to encode PNGs. With that change, we don't need the custom
raster pipeline step, and we can remove that (along with several other
unused functions) from picture_utils. (The one remaining function should
just move to tool_utils, but I'll save that for another rainy day).
In getPixel, we can skip ALL of the processing work, and also only read
the one pixel we need. That makes break-on-change feel much faster from
my anecdotal testing.
Change-Id: I3d18f1e7a15dd12ac4661da1b724e9d8e1cdee96
Reviewed-on: https://skia-review.googlesource.com/146442
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/tools/debugger/SkDrawCommand.cpp b/tools/debugger/SkDrawCommand.cpp
index 27b2009..299a916 100644
--- a/tools/debugger/SkDrawCommand.cpp
+++ b/tools/debugger/SkDrawCommand.cpp
@@ -7,8 +7,6 @@
#include "SkDrawCommand.h"
-#include "png.h"
-
#include "SkAutoMalloc.h"
#include "SkColorFilter.h"
#include "SkDashPathEffect.h"
@@ -19,6 +17,7 @@
#include "SkPaintDefaults.h"
#include "SkPathEffect.h"
#include "SkPicture.h"
+#include "SkPngEncoder.h"
#include "SkReadBuffer.h"
#include "SkRectPriv.h"
#include "SkTextBlobPriv.h"
@@ -721,46 +720,14 @@
sk_free(data);
}
-static void write_png_callback(png_structp png_ptr, png_bytep data, png_size_t length) {
- SkWStream* out = (SkWStream*) png_get_io_ptr(png_ptr);
- out->write(data, length);
-}
+void SkDrawCommand::WritePNG(SkBitmap bitmap, SkWStream& out) {
+ SkPixmap pm;
+ SkAssertResult(bitmap.peekPixels(&pm));
-void SkDrawCommand::WritePNG(const uint8_t* rgba, unsigned width, unsigned height,
- SkWStream& out, bool isOpaque) {
- png_structp png = png_create_write_struct(PNG_LIBPNG_VER_STRING, nullptr, nullptr, nullptr);
- SkASSERT(png != nullptr);
- png_infop info_ptr = png_create_info_struct(png);
- SkASSERT(info_ptr != nullptr);
- if (setjmp(png_jmpbuf(png))) {
- SK_ABORT("png encode error");
- }
- png_set_write_fn(png, &out, write_png_callback, nullptr);
- int colorType = isOpaque ? PNG_COLOR_TYPE_RGB : PNG_COLOR_TYPE_RGBA;
- png_set_IHDR(png, info_ptr, width, height, 8, colorType, PNG_INTERLACE_NONE,
- PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT);
- png_set_compression_level(png, 1);
- png_bytepp rows = (png_bytepp) sk_malloc_throw(height * sizeof(png_byte*));
- png_bytep pixels = (png_bytep) sk_malloc_throw(width * height * 4);
- for (png_size_t y = 0; y < height; ++y) {
- const uint8_t* src = rgba + y * width * 4;
- rows[y] = pixels + y * width * 4;
- for (png_size_t x = 0; x < width; ++x) {
- rows[y][x * 4] = src[x * 4];
- rows[y][x * 4 + 1] = src[x * 4 + 1];
- rows[y][x * 4 + 2] = src[x * 4 + 2];
- rows[y][x * 4 + 3] = src[x * 4 + 3];
- }
- }
- png_write_info(png, info_ptr);
- if (isOpaque) {
- png_set_filler(png, 0xFF, PNG_FILLER_AFTER);
- }
- png_set_filter(png, 0, PNG_NO_FILTERS);
- png_write_image(png, &rows[0]);
- png_destroy_write_struct(&png, nullptr);
- sk_free(rows);
- sk_free(pixels);
+ SkPngEncoder::Options options;
+ options.fZLibLevel = 1;
+ options.fFilterFlags = SkPngEncoder::FilterFlag::kNone;
+ SkPngEncoder::Encode(&out, pm, options);
}
bool SkDrawCommand::flatten(const SkImage& image, Json::Value* target,
@@ -776,11 +743,9 @@
SkBitmap bm;
bm.installPixels(dstInfo, buffer.get(), rowBytes);
- sk_sp<SkData> encodedBitmap = sk_tools::encode_bitmap_for_png(bm);
SkDynamicMemoryWStream out;
- SkDrawCommand::WritePNG(encodedBitmap->bytes(), image.width(), image.height(),
- out, false);
+ SkDrawCommand::WritePNG(bm, out);
sk_sp<SkData> encoded = out.detachAsData();
Json::Value jsonData;
encode_data(encoded->data(), encoded->size(), "image/png", urlDataManager, &jsonData);
diff --git a/tools/debugger/SkDrawCommand.h b/tools/debugger/SkDrawCommand.h
index a7a3015..76d5f06 100644
--- a/tools/debugger/SkDrawCommand.h
+++ b/tools/debugger/SkDrawCommand.h
@@ -73,8 +73,7 @@
static const int kOpTypeCount = kLast_OpType + 1;
- static void WritePNG(const uint8_t* rgba, unsigned width, unsigned height,
- SkWStream& out, bool isOpaque);
+ static void WritePNG(SkBitmap bitmap, SkWStream& out);
SkDrawCommand(OpType opType);
diff --git a/tools/picture_utils.cpp b/tools/picture_utils.cpp
index d0d2198..9a1eb27 100644
--- a/tools/picture_utils.cpp
+++ b/tools/picture_utils.cpp
@@ -7,108 +7,30 @@
#include "picture_utils.h"
#include "SkBitmap.h"
-#include "SkColorPriv.h"
-#include "SkHalf.h"
-#include "SkImageEncoder.h"
#include "SkOSFile.h"
#include "SkOSPath.h"
-#include "SkPM4fPriv.h"
-#include "SkPicture.h"
-#include "SkStream.h"
#include "SkString.h"
-#include "SkRasterPipeline.h"
#include "sk_tool_utils.h"
namespace sk_tools {
- void force_all_opaque(const SkBitmap& bitmap) {
- SkASSERT(kN32_SkColorType == bitmap.colorType());
- if (kN32_SkColorType == bitmap.colorType()) {
- return;
- }
- for (int y = 0; y < bitmap.height(); y++) {
- for (int x = 0; x < bitmap.width(); x++) {
- *bitmap.getAddr32(x, y) |= (SK_A32_MASK << SK_A32_SHIFT);
- }
- }
+bool write_bitmap_to_disk(const SkBitmap& bm, const SkString& dirPath,
+ const char *subdirOrNull, const SkString& baseName) {
+ SkString partialPath;
+ if (subdirOrNull) {
+ partialPath = SkOSPath::Join(dirPath.c_str(), subdirOrNull);
+ sk_mkdir(partialPath.c_str());
+ } else {
+ partialPath.set(dirPath);
}
-
- void replace_char(SkString* str, const char oldChar, const char newChar) {
- if (nullptr == str) {
- return;
- }
- for (size_t i = 0; i < str->size(); ++i) {
- if (oldChar == str->operator[](i)) {
- str->operator[](i) = newChar;
- }
- }
+ SkString fullPath = SkOSPath::Join(partialPath.c_str(), baseName.c_str());
+ if (sk_tool_utils::EncodeImageToFile(fullPath.c_str(), bm, SkEncodedImageFormat::kPNG, 100)) {
+ return true;
+ } else {
+ SkDebugf("Failed to write the bitmap to %s.\n", fullPath.c_str());
+ return false;
}
-
- bool is_percentage(const char* const string) {
- SkString skString(string);
- return skString.endsWith("%");
- }
-
- void setup_bitmap(SkBitmap* bitmap, int width, int height) {
- bitmap->allocN32Pixels(width, height);
- bitmap->eraseColor(SK_ColorTRANSPARENT);
- }
-
- bool write_bitmap_to_disk(const SkBitmap& bm, const SkString& dirPath,
- const char *subdirOrNull, const SkString& baseName) {
- SkString partialPath;
- if (subdirOrNull) {
- partialPath = SkOSPath::Join(dirPath.c_str(), subdirOrNull);
- sk_mkdir(partialPath.c_str());
- } else {
- partialPath.set(dirPath);
- }
- SkString fullPath = SkOSPath::Join(partialPath.c_str(), baseName.c_str());
- if (sk_tool_utils::EncodeImageToFile(fullPath.c_str(), bm, SkEncodedImageFormat::kPNG, 100)) {
- return true;
- } else {
- SkDebugf("Failed to write the bitmap to %s.\n", fullPath.c_str());
- return false;
- }
- }
-
- sk_sp<SkData> encode_bitmap_for_png(SkBitmap bitmap) {
- const int w = bitmap.width(),
- h = bitmap.height();
-
- // PNG wants unpremultiplied 8-bit RGBA pixels (16-bit could work fine too).
- // We leave the gamma of these bytes unspecified, to continue the status quo,
- // which we think generally is to interpret them as sRGB.
-
- SkAutoTMalloc<uint32_t> rgba(w*h);
-
- SkJumper_MemoryCtx src = { bitmap.getPixels(), bitmap.rowBytesAsPixels() },
- dst = { rgba.get(), w };
-
- SkRasterPipeline_<256> p;
- switch (bitmap.colorType()) {
- case kRGBA_F16_SkColorType: p.append(SkRasterPipeline::load_f16, &src); break;
- case kBGRA_8888_SkColorType: p.append(SkRasterPipeline::load_bgra, &src); break;
- case kRGBA_8888_SkColorType: p.append(SkRasterPipeline::load_8888, &src); break;
- case kRGB_565_SkColorType: p.append(SkRasterPipeline::load_565, &src); break;
- default: SkASSERT(false); // DM doesn't support any other formats, does it?
- }
- if (bitmap.info().gammaCloseToSRGB()) {
- p.append(SkRasterPipeline::from_srgb);
- }
- p.append(SkRasterPipeline::unpremul);
- p.append(SkRasterPipeline::clamp_0);
- p.append(SkRasterPipeline::clamp_1);
- if (bitmap.info().colorSpace()) {
- // We leave legacy modes as-is. They're already sRGB encoded (kind of).
- p.append(SkRasterPipeline::to_srgb);
- }
- p.append(SkRasterPipeline::store_8888, &dst);
-
- p.run(0,0, w,h);
-
- return SkData::MakeFromMalloc(rgba.release(), w*h*sizeof(uint32_t));
- }
+}
} // namespace sk_tools
diff --git a/tools/picture_utils.h b/tools/picture_utils.h
index 49a2c82..b990703 100644
--- a/tools/picture_utils.h
+++ b/tools/picture_utils.h
@@ -8,34 +8,10 @@
#ifndef picture_utils_DEFINED
#define picture_utils_DEFINED
-#include "SkBitmap.h"
-
-class SkData;
+class SkBitmap;
class SkString;
namespace sk_tools {
- // since PNG insists on unpremultiplying our alpha, we take no precision
- // chances and force all pixels to be 100% opaque, otherwise on compare we
- // may not get a perfect match.
- //
- // This expects a bitmap with a config type of 8888 and for the pixels to
- // not be on the GPU.
- void force_all_opaque(const SkBitmap& bitmap);
-
- /**
- * Replaces all instances of oldChar with newChar in str.
- */
- void replace_char(SkString* str, const char oldChar, const char newChar);
-
- // Returns true if the string ends with %
- bool is_percentage(const char* const string);
-
- // Prepares the bitmap so that it can be written.
- //
- // Specifically, it configures the bitmap, allocates pixels and then
- // erases the pixels to transparent black.
- void setup_bitmap(SkBitmap* bitmap, int width, int height);
-
/**
* Write a bitmap file to disk.
*
@@ -49,12 +25,6 @@
bool write_bitmap_to_disk(const SkBitmap& bm, const SkString& dirPath,
const char *subdirOrNull, const SkString& baseName);
- // Return raw unpremultiplied RGBA bytes, suitable for storing in a PNG. The output
- // colors are assumed to be sRGB values. This is only guaranteed to work for the
- // cases that are currently emitted by tools:
- // Linear premul 8888, sRGB premul 8888, Linear premul F16
- sk_sp<SkData> encode_bitmap_for_png(SkBitmap bitmap);
-
} // namespace sk_tools
#endif // picture_utils_DEFINED
diff --git a/tools/skiaserve/Request.cpp b/tools/skiaserve/Request.cpp
index 61f79f1..215f667 100644
--- a/tools/skiaserve/Request.cpp
+++ b/tools/skiaserve/Request.cpp
@@ -37,29 +37,15 @@
}
}
-SkBitmap* Request::getBitmapFromCanvas(SkCanvas* canvas) {
- SkBitmap* bmp = new SkBitmap();
- if (!bmp->tryAllocPixels(canvas->imageInfo()) || !canvas->readPixels(*bmp, 0, 0)) {
- fprintf(stderr, "Can't read pixels\n");
- delete bmp;
- return nullptr;
- }
- return bmp;
-}
-
sk_sp<SkData> Request::writeCanvasToPng(SkCanvas* canvas) {
// capture pixels
- std::unique_ptr<SkBitmap> bmp(this->getBitmapFromCanvas(canvas));
- SkASSERT(bmp);
-
- // Convert to format suitable for PNG output
- sk_sp<SkData> encodedBitmap = sk_tools::encode_bitmap_for_png(*bmp);
- SkASSERT(encodedBitmap.get());
+ SkBitmap bmp;
+ bmp.allocPixels(canvas->imageInfo());
+ SkAssertResult(canvas->readPixels(bmp, 0, 0));
// write to an opaque png (black background)
SkDynamicMemoryWStream buffer;
- SkDrawCommand::WritePNG(encodedBitmap->bytes(), bmp->width(), bmp->height(),
- buffer, true);
+ SkDrawCommand::WritePNG(bmp, buffer);
return buffer.detachAsData();
}
@@ -84,14 +70,9 @@
return target;
}
-void Request::drawToCanvas(int n, int m) {
- SkCanvas* target = this->getCanvas();
- fDebugCanvas->drawTo(target, n, m);
-}
-
sk_sp<SkData> Request::drawToPng(int n, int m) {
//fDebugCanvas->setOverdrawViz(true);
- this->drawToCanvas(n, m);
+ fDebugCanvas->drawTo(this->getCanvas(), n, m);
//fDebugCanvas->setOverdrawViz(false);
return writeCanvasToPng(this->getCanvas());
}
@@ -279,16 +260,8 @@
}
SkColor Request::getPixel(int x, int y) {
- SkCanvas* canvas = this->getCanvas();
- canvas->flush();
- std::unique_ptr<SkBitmap> bitmap(this->getBitmapFromCanvas(canvas));
- SkASSERT(bitmap);
-
- // Convert to format suitable for inspection
- sk_sp<SkData> encodedBitmap = sk_tools::encode_bitmap_for_png(*bitmap);
- SkASSERT(encodedBitmap);
-
- const uint8_t* start = encodedBitmap->bytes() + ((y * bitmap->width() + x) * 4);
- SkColor result = SkColorSetARGB(start[3], start[0], start[1], start[2]);
- return result;
+ SkBitmap bmp;
+ bmp.allocPixels(this->getCanvas()->imageInfo().makeWH(1, 1));
+ SkAssertResult(this->getCanvas()->readPixels(bmp, x, y));
+ return bmp.getColor(0, 0);
}
diff --git a/tools/skiaserve/Request.h b/tools/skiaserve/Request.h
index 44df83d..6c970be 100644
--- a/tools/skiaserve/Request.h
+++ b/tools/skiaserve/Request.h
@@ -39,7 +39,6 @@
sk_sp<SkData> drawToPng(int n, int m = -1);
sk_sp<SkData> writeOutSkp();
SkCanvas* getCanvas();
- SkBitmap* getBitmapFromCanvas(SkCanvas* canvas);
bool enableGPU(bool enable);
bool setOverdraw(bool enable);
bool setColorMode(int mode);
@@ -66,7 +65,6 @@
private:
sk_sp<SkData> writeCanvasToPng(SkCanvas* canvas);
- void drawToCanvas(int n, int m = -1);
SkSurface* createCPUSurface();
SkSurface* createGPUSurface();
SkIRect getBounds();