From a8448bc3dfe0f2e768838b4416fb3ebf823b694e Mon Sep 17 00:00:00 2001 From: halcanary Date: Fri, 17 Apr 2015 13:27:24 -0700 Subject: [PATCH] PDF: Correctly embed JPEG images directly into PDF output. We only embed images with YUV planes. That should only grab the subset of color JPEGs supported by PDF. BUG=skia:3180 Review URL: https://codereview.chromium.org/1025773002 --- gm/repeated_bitmap.cpp | 20 +++++++ src/pdf/SkPDFBitmap.cpp | 103 ++++++++++++++++++++++++++++++++----- src/pdf/SkPDFBitmap.h | 11 +--- tests/PDFJpegEmbedTest.cpp | 12 +---- 4 files changed, 113 insertions(+), 33 deletions(-) diff --git a/gm/repeated_bitmap.cpp b/gm/repeated_bitmap.cpp index fa5fe0c136..e6b06365e6 100644 --- a/gm/repeated_bitmap.cpp +++ b/gm/repeated_bitmap.cpp @@ -29,3 +29,23 @@ DEF_SIMPLE_GM(repeated_bitmap, canvas, 576, 576) { } } } + +DEF_SIMPLE_GM(repeated_bitmap_jpg, canvas, 576, 576) { + sk_tool_utils::draw_checkerboard(canvas, 0xFF999999, SK_ColorWHITE, 12); + SkRect rect = SkRect::MakeLTRB(-68.0f, -68.0f, 68.0f, 68.0f); + SkPaint paint; + paint.setColor(0xFF333333); + SkBitmap bm; + if (GetResourceAsBitmap("color_wheel.jpg", &bm)) { + for (int j = 0; j < 4; ++j) { + for (int i = 0; i < 4; ++i) { + SkAutoCanvasRestore autoCanvasRestore(canvas, true); + canvas->translate(96.0f + 192.0f * SkIntToScalar(i), + 96.0f + 192.0f * SkIntToScalar(j)); + canvas->rotate(18.0f * (i + 4 * j)); + canvas->drawRect(rect, paint); + canvas->drawBitmap(bm, -64.0f, -64.0f); + } + } + } +} diff --git a/src/pdf/SkPDFBitmap.cpp b/src/pdf/SkPDFBitmap.cpp index 2a9deb6d0b..3b0d38ce26 100644 --- a/src/pdf/SkPDFBitmap.cpp +++ b/src/pdf/SkPDFBitmap.cpp @@ -6,9 +6,12 @@ */ #include "SkColorPriv.h" +#include "SkData.h" #include "SkFlate.h" +#include "SkImageGenerator.h" #include "SkPDFBitmap.h" #include "SkPDFCanon.h" +#include "SkPixelRef.h" #include "SkStream.h" #include "SkUnPreMultiply.h" @@ -283,8 +286,23 @@ void PDFAlphaBitmap::emitObject(SkWStream* stream, //////////////////////////////////////////////////////////////////////////////// -void SkPDFBitmap::addResources(SkPDFObjNumMap* catalog, - const SkPDFSubstituteMap& substitutes) const { +namespace { +class PDFDefaultBitmap : public SkPDFBitmap { +public: + const SkAutoTUnref fSMask; + void emitObject(SkWStream*, + const SkPDFObjNumMap&, + const SkPDFSubstituteMap&) override; + void addResources(SkPDFObjNumMap*, + const SkPDFSubstituteMap&) const override; + PDFDefaultBitmap(const SkBitmap& bm, SkPDFObject* smask) + : SkPDFBitmap(bm), fSMask(smask) {} +}; +} // namespace + +void PDFDefaultBitmap::addResources( + SkPDFObjNumMap* catalog, + const SkPDFSubstituteMap& substitutes) const { if (fSMask.get()) { SkPDFObject* obj = substitutes.getSubstitute(fSMask.get()); SkASSERT(obj); @@ -324,9 +342,9 @@ static SkPDFArray* make_indexed_color_space(const SkColorTable* table) { return result; } -void SkPDFBitmap::emitObject(SkWStream* stream, - const SkPDFObjNumMap& objNumMap, - const SkPDFSubstituteMap& substitutes) { +void PDFDefaultBitmap::emitObject(SkWStream* stream, + const SkPDFObjNumMap& objNumMap, + const SkPDFSubstituteMap& substitutes) { SkAutoLockPixels autoLockPixels(fBitmap); SkASSERT(fBitmap.colorType() != kIndex_8_SkColorType || fBitmap.getColorTable()); @@ -357,19 +375,13 @@ void SkPDFBitmap::emitObject(SkWStream* stream, } pdfDict.insertName("Filter", "FlateDecode"); pdfDict.insertInt("Length", asset->getLength()); - pdfDict.emitObject(stream, objNumMap,substitutes); + pdfDict.emitObject(stream, objNumMap, substitutes); pdf_stream_begin(stream); stream->writeStream(asset.get(), asset->getLength()); pdf_stream_end(stream); } -SkPDFBitmap::SkPDFBitmap(const SkBitmap& bm, - SkPDFObject* smask) - : fBitmap(bm), fSMask(smask) {} - -SkPDFBitmap::~SkPDFBitmap() {} - //////////////////////////////////////////////////////////////////////////////// static const SkBitmap& immutable_bitmap(const SkBitmap& bm, SkBitmap* copy) { @@ -381,6 +393,59 @@ static const SkBitmap& immutable_bitmap(const SkBitmap& bm, SkBitmap* copy) { return *copy; } +namespace { +/** + * This PDFObject assumes that its constructor was handed YUV JFIF + * Jpeg-encoded data that can be directly embedded into a PDF. + */ +class PDFJpegBitmap : public SkPDFBitmap { +public: + SkAutoTUnref fData; + PDFJpegBitmap(const SkBitmap& bm, SkData* data) + : SkPDFBitmap(bm), fData(SkRef(data)) {} + void emitObject(SkWStream*, + const SkPDFObjNumMap&, + const SkPDFSubstituteMap&) override; +}; + +void PDFJpegBitmap::emitObject(SkWStream* stream, + const SkPDFObjNumMap& objNumMap, + const SkPDFSubstituteMap& substituteMap) { + SkPDFDict pdfDict("XObject"); + pdfDict.insertName("Subtype", "Image"); + pdfDict.insertInt("Width", fBitmap.width()); + pdfDict.insertInt("Height", fBitmap.height()); + pdfDict.insertName("ColorSpace", "DeviceRGB"); + pdfDict.insertInt("BitsPerComponent", 8); + pdfDict.insertName("Filter", "DCTDecode"); + pdfDict.insertInt("ColorTransform", 0); + pdfDict.insertInt("Length", SkToInt(fData->size())); + pdfDict.emitObject(stream, objNumMap, substituteMap); + pdf_stream_begin(stream); + stream->write(fData->data(), fData->size()); + pdf_stream_end(stream); +} +} // namespace + +//////////////////////////////////////////////////////////////////////////////// + +static bool is_jfif_yuv_jpeg(SkData* data) { + const uint8_t bytesZeroToThree[] = {0xFF, 0xD8, 0xFF, 0xE0}; + const uint8_t bytesSixToTen[] = {'J', 'F', 'I', 'F', 0}; + // 0 1 2 3 4 5 6 7 8 9 10 + // FF D8 FF E0 ?? ?? 'J' 'F' 'I' 'F' 00 ... + if (data->size() < 11 || + 0 != memcmp(data->bytes(), bytesZeroToThree, + sizeof(bytesZeroToThree)) || + 0 != memcmp(data->bytes() + 6, bytesSixToTen, sizeof(bytesSixToTen))) { + return false; + } + SkAutoTDelete gen(SkImageGenerator::NewFromData(data)); + SkISize sizes[3]; + // Only YUV JPEG allows access to YUV planes. + return gen && gen->getYUV8Planes(sizes, NULL, NULL, NULL); +} + SkPDFBitmap* SkPDFBitmap::Create(SkPDFCanon* canon, const SkBitmap& bitmap) { SkASSERT(canon); if (!SkColorTypeIsValid(bitmap.colorType()) || @@ -395,11 +460,23 @@ SkPDFBitmap* SkPDFBitmap::Create(SkPDFCanon* canon, const SkBitmap& bitmap) { if (SkPDFBitmap* canonBitmap = canon->findBitmap(bm)) { return SkRef(canonBitmap); } + + if (bm.pixelRef() && bm.pixelRefOrigin().isZero() && + bm.dimensions() == bm.pixelRef()->info().dimensions()) { + // Requires the bitmap to be backed by lazy pixels. + SkAutoTUnref data(bm.pixelRef()->refEncodedData()); + if (data && is_jfif_yuv_jpeg(data)) { + SkPDFBitmap* pdfBitmap = SkNEW_ARGS(PDFJpegBitmap, (bm, data)); + canon->addBitmap(pdfBitmap); + return pdfBitmap; + } + } + SkPDFObject* smask = NULL; if (!bm.isOpaque() && !SkBitmap::ComputeIsOpaque(bm)) { smask = SkNEW_ARGS(PDFAlphaBitmap, (bm)); } - SkPDFBitmap* pdfBitmap = SkNEW_ARGS(SkPDFBitmap, (bm, smask)); + SkPDFBitmap* pdfBitmap = SkNEW_ARGS(PDFDefaultBitmap, (bm, smask)); canon->addBitmap(pdfBitmap); return pdfBitmap; } diff --git a/src/pdf/SkPDFBitmap.h b/src/pdf/SkPDFBitmap.h index bf41f6341a..2c8653f315 100644 --- a/src/pdf/SkPDFBitmap.h +++ b/src/pdf/SkPDFBitmap.h @@ -26,22 +26,15 @@ class SkPDFBitmap : public SkPDFObject { public: // Returns NULL on unsupported bitmap; static SkPDFBitmap* Create(SkPDFCanon*, const SkBitmap&); - ~SkPDFBitmap(); - void emitObject(SkWStream*, - const SkPDFObjNumMap& objNumMap, - const SkPDFSubstituteMap& substitutes) override; - void addResources(SkPDFObjNumMap*, - const SkPDFSubstituteMap&) const override; bool equals(const SkBitmap& other) const { return fBitmap.getGenerationID() == other.getGenerationID() && fBitmap.pixelRefOrigin() == other.pixelRefOrigin() && fBitmap.dimensions() == other.dimensions(); } -private: +protected: const SkBitmap fBitmap; - const SkAutoTUnref fSMask; - SkPDFBitmap(const SkBitmap&, SkPDFObject*); + SkPDFBitmap(const SkBitmap& bm) : fBitmap(bm) {} }; #endif // SkPDFBitmap_DEFINED diff --git a/tests/PDFJpegEmbedTest.cpp b/tests/PDFJpegEmbedTest.cpp index beb949dfc9..133d84a3ff 100644 --- a/tests/PDFJpegEmbedTest.cpp +++ b/tests/PDFJpegEmbedTest.cpp @@ -81,19 +81,9 @@ DEF_TEST(PDFJpegEmbedTest, r) { SkASSERT(pdfData); pdf.reset(); - // Test disabled, waiting on resolution to http://skbug.com/3180 - // REPORTER_ASSERT(r, is_subset_of(mandrillData, pdfData)); + REPORTER_ASSERT(r, is_subset_of(mandrillData, pdfData)); // This JPEG uses a nonstandard colorspace - it can not be // embedded into the PDF directly. REPORTER_ASSERT(r, !is_subset_of(cmykData, pdfData)); - - // The following is for debugging purposes only. - const char* outputPath = getenv("SKIA_TESTS_PDF_JPEG_EMBED_OUTPUT_PATH"); - if (outputPath) { - SkFILEWStream output(outputPath); - if (output.isValid()) { - output.write(pdfData->data(), pdfData->size()); - } - } }