aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar halcanary <halcanary@google.com>2014-06-26 14:00:31 -0700
committerGravatar Commit bot <commit-bot@chromium.org>2014-06-26 14:00:31 -0700
commitc1dfa14b645ae274780f026dd86c9b633fbdad06 (patch)
tree1ede8aa602639bb9d7aafbf9b39d8061814999b2
parent11a005ee01973da543fee66f9b14d20a7c407611 (diff)
Switch SkPDFStream's internal storage from SkStream to SkData
Motivation: This makes SkPDFStream thread-safe for two threads serializing it at once, since a SkStream has an internal position. Updated SkPDFFont, SkPDFGraphicState, and SkPDFPage's use of SkPDFStream to use the SkData constructor rather than the SkStream constructor (saving a memcpy). BUG=skia:2683 R=mtklein@google.com, djsollen@google.com Author: halcanary@google.com Review URL: https://codereview.chromium.org/340783013
-rw-r--r--gyp/pdf.gyp1
-rw-r--r--src/pdf/SkPDFFont.cpp71
-rw-r--r--src/pdf/SkPDFGraphicState.cpp7
-rw-r--r--src/pdf/SkPDFImage.cpp10
-rw-r--r--src/pdf/SkPDFPage.cpp4
-rw-r--r--src/pdf/SkPDFStream.cpp37
-rw-r--r--src/pdf/SkPDFStream.h19
7 files changed, 86 insertions, 63 deletions
diff --git a/gyp/pdf.gyp b/gyp/pdf.gyp
index e5923e7483..ca4d52e07f 100644
--- a/gyp/pdf.gyp
+++ b/gyp/pdf.gyp
@@ -18,6 +18,7 @@
'../src/core', # needed to get SkGlyphCache.h and SkTextFormatParams.h
'../src/pdf',
'../src/utils', # needed to get SkBitSet.h
+ '../src/images', # needed to get SkStreamHelpers.h
],
'sources': [
'pdf.gypi', # Makes the gypi appear in IDEs (but does not modify the build).
diff --git a/src/pdf/SkPDFFont.cpp b/src/pdf/SkPDFFont.cpp
index 014b328030..aa4b4b5534 100644
--- a/src/pdf/SkPDFFont.cpp
+++ b/src/pdf/SkPDFFont.cpp
@@ -150,8 +150,8 @@ int8_t hexToBin(uint8_t c) {
return -1;
}
-SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
- size_t* dataLen, size_t* trailerLen) {
+static SkData* handle_type1_stream(SkStream* srcStream, size_t* headerLen,
+ size_t* dataLen, size_t* trailerLen) {
// srcStream may be backed by a file or a unseekable fd, so we may not be
// able to use skip(), rewind(), or getMemoryBase(). read()ing through
// the input only once is doable, but very ugly. Furthermore, it'd be nice
@@ -199,26 +199,43 @@ SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
SkAutoDataUnref aud(data);
if (parsePFB(src, srcLen, headerLen, dataLen, trailerLen)) {
- SkMemoryStream* result =
- new SkMemoryStream(*headerLen + *dataLen + *trailerLen);
- memcpy((char*)result->getAtPos(), src + 6, *headerLen);
- result->seek(*headerLen);
- memcpy((char*)result->getAtPos(), src + 6 + *headerLen + 6, *dataLen);
- result->seek(*headerLen + *dataLen);
- memcpy((char*)result->getAtPos(), src + 6 + *headerLen + 6 + *dataLen,
- *trailerLen);
- result->rewind();
- return result;
+ static const int kPFBSectionHeaderLength = 6;
+ const size_t length = *headerLen + *dataLen + *trailerLen;
+ SkASSERT(length > 0);
+ SkASSERT(length + (2 * kPFBSectionHeaderLength) <= srcLen);
+
+ SkAutoTMalloc<uint8_t> buffer(length);
+
+ const uint8_t* const srcHeader = src + kPFBSectionHeaderLength;
+ // There is a six-byte section header before header and data
+ // (but not trailer) that we're not going to copy.
+ const uint8_t* const srcData
+ = srcHeader + *headerLen + kPFBSectionHeaderLength;
+ const uint8_t* const srcTrailer = srcData + *headerLen;
+
+ uint8_t* const resultHeader = buffer.get();
+ uint8_t* const resultData = resultHeader + *headerLen;
+ uint8_t* const resultTrailer = resultData + *dataLen;
+
+ SkASSERT(resultTrailer + *trailerLen == resultHeader + length);
+
+ memcpy(resultHeader, srcHeader, *headerLen);
+ memcpy(resultData, srcData, *dataLen);
+ memcpy(resultTrailer, srcTrailer, *trailerLen);
+
+ return SkData::NewFromMalloc(buffer.detach(), length);
}
// A PFA has to be converted for PDF.
size_t hexDataLen;
if (parsePFA((const char*)src, srcLen, headerLen, &hexDataLen, dataLen,
trailerLen)) {
- SkMemoryStream* result =
- new SkMemoryStream(*headerLen + *dataLen + *trailerLen);
- memcpy((char*)result->getAtPos(), src, *headerLen);
- result->seek(*headerLen);
+ const size_t length = *headerLen + *dataLen + *trailerLen;
+ SkASSERT(length > 0);
+ SkAutoTMalloc<uint8_t> buffer(length);
+
+ memcpy(buffer.get(), src, *headerLen);
+ uint8_t* const resultData = &(buffer[*headerLen]);
const uint8_t* hexData = src + *headerLen;
const uint8_t* trailer = hexData + hexDataLen;
@@ -236,21 +253,19 @@ SkStream* handleType1Stream(SkStream* srcStream, size_t* headerLen,
} else {
dataByte |= curNibble;
highNibble = true;
- ((char *)result->getAtPos())[outputOffset++] = dataByte;
+ resultData[outputOffset++] = dataByte;
}
}
if (!highNibble) {
- ((char *)result->getAtPos())[outputOffset++] = dataByte;
+ resultData[outputOffset++] = dataByte;
}
SkASSERT(outputOffset == *dataLen);
- result->seek(*headerLen + outputOffset);
- memcpy((char *)result->getAtPos(), src + *headerLen + hexDataLen,
- *trailerLen);
- result->rewind();
- return result;
- }
+ uint8_t* const resultTrailer = &(buffer[*headerLen + outputOffset]);
+ memcpy(resultTrailer, src + *headerLen + hexDataLen, *trailerLen);
+ return SkData::NewFromMalloc(buffer.detach(), length);
+ }
return NULL;
}
@@ -556,9 +571,8 @@ static SkPDFStream* generate_tounicode_cmap(
append_cmap_sections(glyphToUnicode, subset, &cmap, multiByteGlyphs,
firstGlyphID, lastGlyphID);
append_cmap_footer(&cmap);
- SkAutoTUnref<SkMemoryStream> cmapStream(new SkMemoryStream());
- cmapStream->setData(cmap.copyToData())->unref();
- return new SkPDFStream(cmapStream.get());
+ SkAutoTUnref<SkData> cmapData(cmap.copyToData());
+ return new SkPDFStream(cmapData.get());
}
#if defined (SK_SFNTLY_SUBSETTER)
@@ -574,6 +588,7 @@ static size_t get_subset_font_stream(const char* fontName,
SkPDFStream** fontStream) {
int ttcIndex;
SkAutoTUnref<SkStream> fontData(typeface->openStream(&ttcIndex));
+ SkASSERT(fontData.get());
size_t fontSize = fontData->getLength();
@@ -1295,7 +1310,7 @@ bool SkPDFType1Font::addFontDescriptor(int16_t defaultWidth) {
size_t data SK_INIT_TO_AVOID_WARNING;
size_t trailer SK_INIT_TO_AVOID_WARNING;
SkAutoTUnref<SkStream> rawFontData(typeface()->openStream(&ttcIndex));
- SkStream* fontData = handleType1Stream(rawFontData.get(), &header, &data,
+ SkData* fontData = handle_type1_stream(rawFontData.get(), &header, &data,
&trailer);
if (fontData == NULL) {
return false;
diff --git a/src/pdf/SkPDFGraphicState.cpp b/src/pdf/SkPDFGraphicState.cpp
index 1b495341b9..fa3baaf95e 100644
--- a/src/pdf/SkPDFGraphicState.cpp
+++ b/src/pdf/SkPDFGraphicState.cpp
@@ -5,10 +5,10 @@
* found in the LICENSE file.
*/
+#include "SkData.h"
#include "SkPDFFormXObject.h"
#include "SkPDFGraphicState.h"
#include "SkPDFUtils.h"
-#include "SkStream.h"
#include "SkTypes.h"
static const char* blend_mode_from_xfermode(SkXfermode::Mode mode) {
@@ -121,8 +121,9 @@ SkPDFObject* SkPDFGraphicState::GetInvertFunction() {
domainAndRange->appendInt(1);
static const char psInvert[] = "{1 exch sub}";
- SkAutoTUnref<SkMemoryStream> psInvertStream(
- new SkMemoryStream(&psInvert, strlen(psInvert), true));
+ // Do not copy the trailing '\0' into the SkData.
+ SkAutoTUnref<SkData> psInvertStream(
+ SkData::NewWithCopy(psInvert, strlen(psInvert)));
invertFunction = new SkPDFStream(psInvertStream.get());
invertFunction->insertInt("FunctionType", 4);
diff --git a/src/pdf/SkPDFImage.cpp b/src/pdf/SkPDFImage.cpp
index 77fd84eff9..7e17f98a15 100644
--- a/src/pdf/SkPDFImage.cpp
+++ b/src/pdf/SkPDFImage.cpp
@@ -512,7 +512,7 @@ SkPDFImage::SkPDFImage(SkStream* stream,
}
if (stream != NULL) {
- setData(stream);
+ this->setData(stream);
fStreamValid = true;
} else {
fStreamValid = false;
@@ -598,13 +598,11 @@ bool SkPDFImage::populate(SkPDFCatalog* catalog) {
SkAutoTUnref<SkData> data(fEncoder(&pixelRefOffset, subset));
if (data.get() && data->size() < get_uncompressed_size(fBitmap,
fSrcRect)) {
- SkAutoTUnref<SkStream> stream(SkNEW_ARGS(SkMemoryStream,
- (data)));
- setData(stream.get());
+ this->setData(data.get());
insertName("Filter", "DCTDecode");
insertInt("ColorTransform", kNoColorTransform);
- insertInt("Length", getData()->getLength());
+ insertInt("Length", this->dataSize());
setState(kCompressed_State);
return true;
}
@@ -613,7 +611,7 @@ bool SkPDFImage::populate(SkPDFCatalog* catalog) {
if (!fStreamValid) {
SkAutoTUnref<SkStream> stream(
extract_image_data(fBitmap, fSrcRect, fIsAlpha, NULL));
- setData(stream);
+ this->setData(stream);
fStreamValid = true;
}
return INHERITED::populate(catalog);
diff --git a/src/pdf/SkPDFPage.cpp b/src/pdf/SkPDFPage.cpp
index 8961d2f3d3..cfb6790876 100644
--- a/src/pdf/SkPDFPage.cpp
+++ b/src/pdf/SkPDFPage.cpp
@@ -7,11 +7,11 @@
*/
+#include "SkData.h"
#include "SkPDFCatalog.h"
#include "SkPDFDevice.h"
#include "SkPDFPage.h"
#include "SkPDFResourceDict.h"
-#include "SkStream.h"
SkPDFPage::SkPDFPage(SkPDFDevice* content)
: SkPDFDict("Page"),
@@ -36,7 +36,7 @@ void SkPDFPage::finalizePage(SkPDFCatalog* catalog, bool firstPage,
}
}
- SkAutoTUnref<SkStream> content(fDevice->content());
+ SkAutoTUnref<SkData> content(fDevice->copyContentToData());
fContentStream.reset(new SkPDFStream(content.get()));
insert("Contents", new SkPDFObjRef(fContentStream.get()))->unref();
}
diff --git a/src/pdf/SkPDFStream.cpp b/src/pdf/SkPDFStream.cpp
index e570976403..319433f7d0 100644
--- a/src/pdf/SkPDFStream.cpp
+++ b/src/pdf/SkPDFStream.cpp
@@ -12,6 +12,7 @@
#include "SkPDFCatalog.h"
#include "SkPDFStream.h"
#include "SkStream.h"
+#include "SkStreamHelpers.h" // CopyStreamToData
static bool skip_compression(SkPDFCatalog* catalog) {
return SkToBool(catalog->getDocumentFlags() &
@@ -19,17 +20,17 @@ static bool skip_compression(SkPDFCatalog* catalog) {
}
SkPDFStream::SkPDFStream(SkStream* stream) : fState(kUnused_State) {
- setData(stream);
+ this->setData(stream);
}
SkPDFStream::SkPDFStream(SkData* data) : fState(kUnused_State) {
- setData(data);
+ this->setData(data);
}
SkPDFStream::SkPDFStream(const SkPDFStream& pdfStream)
: SkPDFDict(),
fState(kUnused_State) {
- setData(pdfStream.fData.get());
+ this->setData(pdfStream.fData.get());
bool removeLength = true;
// Don't uncompress an already compressed stream, but we could.
if (pdfStream.fState == kCompressed_State) {
@@ -55,14 +56,16 @@ void SkPDFStream::emitObject(SkWStream* stream, SkPDFCatalog* catalog,
if (indirect) {
return emitIndirectObject(stream, catalog);
}
+ SkAutoMutexAcquire lock(fMutex); // multiple threads could be calling emit
if (!this->populate(catalog)) {
return fSubstitute->emitObject(stream, catalog, indirect);
}
this->INHERITED::emitObject(stream, catalog, false);
stream->writeText(" stream\n");
- stream->writeStream(fData.get(), fData->getLength());
- fData->rewind();
+ if (fData.get()) {
+ stream->write(fData->data(), fData->size());
+ }
stream->writeText("\nendstream");
}
@@ -70,30 +73,34 @@ size_t SkPDFStream::getOutputSize(SkPDFCatalog* catalog, bool indirect) {
if (indirect) {
return getIndirectOutputSize(catalog);
}
+ SkAutoMutexAcquire lock(fMutex); // multiple threads could be calling emit
if (!this->populate(catalog)) {
return fSubstitute->getOutputSize(catalog, indirect);
}
return this->INHERITED::getOutputSize(catalog, false) +
- strlen(" stream\n\nendstream") + fData->getLength();
+ strlen(" stream\n\nendstream") + this->dataSize();
}
SkPDFStream::SkPDFStream() : fState(kUnused_State) {}
void SkPDFStream::setData(SkData* data) {
- SkMemoryStream* stream = new SkMemoryStream;
- stream->setData(data);
- fData.reset(stream); // Transfer ownership.
+ fData.reset(SkSafeRef(data));
}
void SkPDFStream::setData(SkStream* stream) {
// Code assumes that the stream starts at the beginning and is rewindable.
if (stream) {
SkASSERT(stream->getPosition() == 0);
+ fData.reset(CopyStreamToData(stream));
SkASSERT(stream->rewind());
+ } else {
+ fData.reset(NULL);
}
- fData.reset(stream);
- SkSafeRef(stream);
+}
+
+size_t SkPDFStream::dataSize() const {
+ return fData.get() ? fData->size() : 0;
}
bool SkPDFStream::populate(SkPDFCatalog* catalog) {
@@ -102,17 +109,15 @@ bool SkPDFStream::populate(SkPDFCatalog* catalog) {
SkDynamicMemoryWStream compressedData;
SkAssertResult(SkFlate::Deflate(fData.get(), &compressedData));
- if (compressedData.getOffset() < fData->getLength()) {
- SkMemoryStream* stream = new SkMemoryStream;
- stream->setData(compressedData.copyToData())->unref();
- fData.reset(stream); // Transfer ownership.
+ if (compressedData.getOffset() < this->dataSize()) {
+ fData.reset(compressedData.copyToData());
insertName("Filter", "FlateDecode");
}
fState = kCompressed_State;
} else {
fState = kNoCompression_State;
}
- insertInt("Length", fData->getLength());
+ insertInt("Length", this->dataSize());
} else if (fState == kNoCompression_State && !skip_compression(catalog) &&
SkFlate::HaveFlate()) {
if (!fSubstitute.get()) {
diff --git a/src/pdf/SkPDFStream.h b/src/pdf/SkPDFStream.h
index 6371bc187d..f847a2952b 100644
--- a/src/pdf/SkPDFStream.h
+++ b/src/pdf/SkPDFStream.h
@@ -41,7 +41,8 @@ public:
explicit SkPDFStream(const SkPDFStream& pdfStream);
virtual ~SkPDFStream();
- // The SkPDFObject interface.
+ // The SkPDFObject interface. These two methods use a mutex to
+ // allow multiple threads to call at the same time.
virtual void emitObject(SkWStream* stream, SkPDFCatalog* catalog,
bool indirect);
virtual size_t getOutputSize(SkPDFCatalog* catalog, bool indirect);
@@ -67,22 +68,22 @@ protected:
fSubstitute.reset(stream);
}
- SkPDFStream* getSubstitute() {
+ SkPDFStream* getSubstitute() const {
return fSubstitute.get();
}
void setData(SkData* data);
void setData(SkStream* stream);
- SkStream* getData() {
- return fData.get();
- }
+ size_t dataSize() const;
+
+ SkData* getData() const { return fData.get(); }
void setState(State state) {
fState = state;
}
- State getState() {
+ State getState() const {
return fState;
}
@@ -90,8 +91,10 @@ private:
// Indicates what form (or if) the stream has been requested.
State fState;
- // TODO(vandebo): Use SkData (after removing deprecated constructor).
- SkAutoTUnref<SkStream> fData;
+ // Mutex guards fState, fData, and fSubstitute in public interface.
+ SkMutex fMutex;
+
+ SkAutoTUnref<SkData> fData;
SkAutoTUnref<SkPDFStream> fSubstitute;
typedef SkPDFDict INHERITED;