Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1211)

Unified Diff: third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp

Issue 2618633004: Add support for Animated PNG (Closed)
Patch Set: Reject bad data. Cleanups Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
index ea896bd0370b078d13b7eb7b5563712d6da8fd1c..8cf8571ffff84690d5a37a6008e07643145476d0 100644
--- a/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/png/PNGImageDecoder.cpp
@@ -38,9 +38,7 @@
#include "platform/image-decoders/png/PNGImageDecoder.h"
-#include "platform/image-decoders/png/PNGImageReader.h"
#include "png.h"
-#include "wtf/PtrUtil.h"
#include <memory>
Noel Gordon 2017/02/21 13:53:54 Remove these two #include, PNGImageDecoder.h provi
scroggo_chromium 2017/02/23 22:09:52 Done.
namespace blink {
@@ -50,10 +48,110 @@ PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption,
size_t maxDecodedBytes,
size_t offset)
: ImageDecoder(alphaOption, colorBehavior, maxDecodedBytes),
- m_offset(offset) {}
+ m_offset(offset),
+ m_currentFrame(0),
+ // It might be more logical to default to cAnimationNone, but BitmapImage
Noel Gordon 2017/02/21 13:53:55 // It would be logical to default ...
scroggo_chromium 2017/02/23 22:09:53 Done.
+ // uses that as a signal to never check again, meaning the actual count
+ // will never be respected.
+ m_repetitionCount(cAnimationLoopOnce),
+ m_hasAlphaChannel(false),
+ m_currentBufferSawAlpha(false) {}
PNGImageDecoder::~PNGImageDecoder() {}
+bool PNGImageDecoder::setFailed() {
+ m_reader.reset();
+ return ImageDecoder::setFailed();
+}
+
+size_t PNGImageDecoder::decodeFrameCount() {
+ parse(PNGImageReader::PNGParseQuery::PNGMetaDataQuery);
Noel Gordon 2017/02/21 13:53:54 parse(ParseQuery::MetaData);
scroggo_chromium 2017/02/23 22:09:53 Done.
+ return failed() ? m_frameBufferCache.size() : m_reader->frameCount();
+}
+
+void PNGImageDecoder::decode(size_t index) {
+ parse(PNGImageReader::PNGParseQuery::PNGMetaDataQuery);
Noel Gordon 2017/02/21 13:53:55 parse(ParseQuery::MetaData);
scroggo_chromium 2017/02/23 22:09:52 Done.
+
+ if (failed())
+ return;
+
+ updateAggressivePurging(index);
+
+ Vector<size_t> framesToDecode = findFramesToDecode(index);
+ for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); i++) {
+ m_currentFrame = *i;
+ if (!m_reader->decode(*m_data, *i)) {
+ setFailed();
+ return;
+ }
+
+ // If this returns false, we need more data to continue decoding.
+ if (!postDecodeProcessing(*i))
+ break;
+ }
+
+ // It is also a fatal error if all data is received and we have decoded all
+ // frames available but the file is truncated.
+ if (index >= m_frameBufferCache.size() - 1 && isAllDataReceived() &&
+ m_reader && !m_reader->parseCompleted())
+ setFailed();
+}
+
Noel Gordon 2017/02/21 13:53:55 Move the parse() routine definition to here.
scroggo_chromium 2017/02/23 22:09:53 Done. For my edification, why is here better?
Noel Gordon 2017/02/27 07:45:31 It's then adjacent to the functions that use it (s
+void PNGImageDecoder::clearFrameBuffer(size_t frameIndex) {
Noel Gordon 2017/02/21 13:53:55 frameIndex -> index.
scroggo_chromium 2017/02/23 22:09:53 Done.
+ if (m_reader)
+ m_reader->clearDecodeState(frameIndex);
+ ImageDecoder::clearFrameBuffer(frameIndex);
+}
+
+bool PNGImageDecoder::onInitFrameBuffer(size_t index) {
+ if (PNG_INTERLACE_ADAM7 ==
Noel Gordon 2017/02/21 13:53:54 Let's prefer the early return: png_byte interla
scroggo_chromium 2017/02/23 22:09:52 Done.
+ png_get_interlace_type(m_reader->pngPtr(), m_reader->infoPtr())) {
+ unsigned colorChannels = m_hasAlphaChannel ? 4 : 3;
+ m_reader->createInterlaceBuffer(colorChannels * size().width() *
Noel Gordon 2017/02/21 13:53:55 colorChannels * size().area() ?
scroggo_chromium 2017/02/23 22:09:52 Done.
+ size().height());
+ return m_reader->interlaceBuffer();
Noel Gordon 2017/02/21 13:53:55 If this failed, setFailed() will be called in Imag
scroggo_chromium 2017/02/23 22:09:52 setFailed() can be called in other ways from initF
Noel Gordon 2017/02/27 07:45:31 Yeah, possible security problems.
+ }
+ return true;
+}
+
+bool PNGImageDecoder::canReusePreviousFrameBuffer(size_t frameIndex) const {
Noel Gordon 2017/02/21 13:53:55 frameIndex -> index.
scroggo_chromium 2017/02/23 22:09:53 Done.
+ DCHECK(frameIndex < m_frameBufferCache.size());
+ return m_frameBufferCache[frameIndex].getDisposalMethod() !=
+ ImageFrame::DisposeOverwritePrevious;
+}
+
+void PNGImageDecoder::parse(PNGImageReader::PNGParseQuery query) {
Noel Gordon 2017/02/21 13:53:55 void PNGImageDecoder::parse(ParseQuery query) {
scroggo_chromium 2017/02/23 22:09:54 Done.
+ if (failed() || (m_reader && m_reader->parseCompleted()))
+ return;
+
+ if (!m_reader)
+ m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset);
+
+ if (!m_reader->parse(*m_data, query))
+ setFailed();
+}
+
+void PNGImageDecoder::setRepetitionCount(int repetitionCount) {
+ m_repetitionCount = repetitionCount;
+}
+
+int PNGImageDecoder::repetitionCount() const {
+ return failed() ? cAnimationLoopOnce : m_repetitionCount;
+}
+
+void PNGImageDecoder::initializeNewFrame(size_t index) {
+ const PNGImageReader::FrameInfo& frameInfo = m_reader->frameInfo(index);
+ ImageFrame* buffer = &m_frameBufferCache[index];
Noel Gordon 2017/02/21 13:53:55 ImageFrame& buffer = m_frameBufferCache[index];
scroggo_chromium 2017/02/23 22:09:53 Done.
+
+ DCHECK(IntRect(IntPoint(), size()).contains(frameInfo.frameRect));
+ buffer->setOriginalFrameRect(frameInfo.frameRect);
Noel Gordon 2017/02/21 13:53:54 "buffer->X" to "buffer.X" all through here.
scroggo_chromium 2017/02/23 22:09:52 Done.
+ buffer->setDuration(frameInfo.duration);
Noel Gordon 2017/02/21 13:53:54 Space before this line.
scroggo_chromium 2017/02/23 22:09:53 Done.
+ buffer->setDisposalMethod(frameInfo.disposalMethod);
+ buffer->setAlphaBlendSource(frameInfo.alphaBlend);
+ buffer->setRequiredPreviousFrameIndex(
Noel Gordon 2017/02/21 13:53:54 Space before 151. size_t previousFrameIndex = f
scroggo_chromium 2017/02/23 22:09:53 Done.
+ findRequiredPreviousFrame(index, false));
+}
+
inline float pngFixedToFloat(png_fixed_point x) {
return ((float)x) * 0.00001f;
}
@@ -111,22 +209,8 @@ inline sk_sp<SkColorSpace> readColorSpace(png_structp png, png_infop info) {
void PNGImageDecoder::headerAvailable() {
png_structp png = m_reader->pngPtr();
png_infop info = m_reader->infoPtr();
- png_uint_32 width = png_get_image_width(png, info);
- png_uint_32 height = png_get_image_height(png, info);
-
- // Protect against large PNGs. See http://bugzil.la/251381 for more details.
- const unsigned long maxPNGSize = 1000000UL;
- if (width > maxPNGSize || height > maxPNGSize) {
- longjmp(JMPBUF(png), 1);
- return;
- }
-
- // Set the image size now that the image header is available.
- if (!setSize(width, height)) {
- longjmp(JMPBUF(png), 1);
- return;
- }
+ png_uint_32 width, height;
int bitDepth, colorType, interlaceType, compressionType, filterType, channels;
png_get_IHDR(png, info, &width, &height, &bitDepth, &colorType,
&interlaceType, &compressionType, &filterType);
@@ -152,16 +236,35 @@ void PNGImageDecoder::headerAvailable() {
colorType == PNG_COLOR_TYPE_GRAY_ALPHA)
png_set_gray_to_rgb(png);
- if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) {
- // We only support color profiles for color PALETTE and RGB[A] PNG.
- // Supporting color profiles for gray-scale images is slightly tricky, at
- // least using the CoreGraphics ICC library, because we expand gray-scale
- // images to RGB but we do not similarly transform the color profile. We'd
- // either need to transform the color profile or we'd need to decode into a
- // gray-scale image buffer and hand that to CoreGraphics.
- sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info);
- if (colorSpace) {
- setEmbeddedColorSpace(colorSpace);
+ // Only set the size and the color space of the image once. Since non-first
Noel Gordon 2017/02/21 13:53:54 // Only set the size and the color space of the im
scroggo_chromium 2017/02/23 22:09:53 Done.
+ // frames also use this method, we don't want them to override the size of
+ // the image to the size of their frame rect. Frames also don't specify their
+ // own color space, so we need to set it only once.
+ if (!isDecodedSizeAvailable()) {
+ // Protect against large PNGs. See http://bugzil.la/251381 for more details.
+ const unsigned long maxPNGSize = 1000000UL;
+ if (width > maxPNGSize || height > maxPNGSize) {
+ longjmp(JMPBUF(png), 1);
+ return;
+ }
+
+ if (!setSize(width, height)) {
Noel Gordon 2017/02/21 13:53:55 // Set the image size now that the image header is
scroggo_chromium 2017/02/23 22:09:52 Done.
+ longjmp(JMPBUF(png), 1);
+ return;
+ }
+
+ if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) {
+ // We only support color profiles for color PALETTE and RGB[A] PNG.
+ // Supporting color profiles for gray-scale images is slightly tricky, at
+ // least using the CoreGraphics ICC library, because we expand gray-scale
+ // images to RGB but we do not similarly transform the color profile. We'd
+ // either need to transform the color profile or we'd need to decode into
+ // a
+ // gray-scale image buffer and hand that to CoreGraphics.
+ sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info);
+ if (colorSpace) {
Noel Gordon 2017/02/21 13:53:54 Is sk_sp<T> convertible to bool? If so, we can us
scroggo_chromium 2017/02/23 22:09:52 Addressed by crrev.com/2716533002
+ setEmbeddedColorSpace(colorSpace);
+ }
}
}
Noel Gordon 2017/02/21 13:53:55 Add a DCHECK(isDecodedSizeAvailable()) around here
scroggo_chromium 2017/02/23 22:09:53 Done.
Noel Gordon 2017/02/27 07:45:31 One second thoughts, this is in the middle of some
scroggo_chromium 2017/02/27 20:31:05 As I look for a better place to put it, I think th
Noel Gordon 2017/03/01 09:11:34 Yeah, seem so.
scroggo_chromium 2017/03/03 21:56:40 Done.
@@ -197,99 +300,77 @@ void PNGImageDecoder::headerAvailable() {
// Update our info now.
Noel Gordon 2017/02/21 13:53:54 // Update our info now (so we can read color chann
scroggo_chromium 2017/02/23 22:09:53 Done.
png_read_update_info(png, info);
channels = png_get_channels(png, info);
- ASSERT(channels == 3 || channels == 4);
-
- m_reader->setHasAlpha(channels == 4);
-
- if (m_reader->decodingSizeOnly()) {
-// If we only needed the size, halt the reader.
-#if PNG_LIBPNG_VER_MAJOR > 1 || \
- (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5)
- // Passing '0' tells png_process_data_pause() not to cache unprocessed data.
- m_reader->setReadOffset(m_reader->currentBufferSize() -
- png_process_data_pause(png, 0));
-#else
- m_reader->setReadOffset(m_reader->currentBufferSize() - png->buffer_size);
- png->buffer_size = 0;
-#endif
- }
+ DCHECK(channels == 3 || channels == 4);
+
+ m_hasAlphaChannel = (channels == 4);
+ m_currentBufferSawAlpha = false;
Noel Gordon 2017/02/21 13:53:54 Should this m_currentBufferSawAlpha = false be mov
scroggo_chromium 2017/02/23 22:09:52 Done.
}
void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
unsigned rowIndex,
int) {
- if (m_frameBufferCache.isEmpty())
+ if (m_currentFrame >= m_frameBufferCache.size())
return;
- // Initialize the framebuffer if needed.
- ImageFrame& buffer = m_frameBufferCache[0];
- if (buffer.getStatus() == ImageFrame::FrameEmpty) {
- png_structp png = m_reader->pngPtr();
- if (!buffer.setSizeAndColorSpace(size().width(), size().height(),
- colorSpaceForSkImages())) {
- longjmp(JMPBUF(png), 1);
- return;
- }
+ // When a client calls ImageDecoder::setMemoryAllocator *before* decoding the
+ // frame count, the first frame won't get initialized correctly. The call will
+ // resize |m_frameBufferCache| to 1, and therefore ImageDecoder::frameCount
+ // will *not* call initializeNewFrame for the first frame, whether it is a
+ // static image or the first frame of an animated image. Amongst others, the
+ // frame rect will not be set. If this is the case, initialize the frame here.
+ if (m_frameBufferCache[0].originalFrameRect().size() == IntSize(0, 0))
Noel Gordon 2017/02/21 13:53:54 m_frameBufferCache[0].originalFrameRect().size().i
scroggo_chromium 2017/02/23 22:09:53 This was caught by the following tests: DeferredIm
Noel Gordon 2017/02/27 07:45:31 OK good, we have tests.
scroggo_chromium 2017/02/27 20:31:05 Agreed - GIF decoder should update its alpha state
Noel Gordon 2017/03/01 09:19:35 Nod.
scroggo_chromium 2017/03/03 21:56:40 A separate change fixes frameOpacity, which is als
Noel Gordon 2017/03/06 03:06:45 OK, the "separate change" change can be landed her
+ initializeNewFrame(0);
- unsigned colorChannels = m_reader->hasAlpha() ? 4 : 3;
- if (PNG_INTERLACE_ADAM7 ==
- png_get_interlace_type(png, m_reader->infoPtr())) {
- m_reader->createInterlaceBuffer(colorChannels * size().width() *
- size().height());
- if (!m_reader->interlaceBuffer()) {
- longjmp(JMPBUF(png), 1);
- return;
- }
- }
+ if (!initFrameBuffer(m_currentFrame))
Noel Gordon 2017/02/21 13:53:54 First issue: code in diff-left short circuited rea
scroggo_chromium 2017/02/23 22:09:54 What is the real work you're concerned about here?
scroggo_chromium 2017/02/24 14:57:25 I moved initFrameBuffer into decode in patch set 8
Noel Gordon 2017/02/27 07:45:31 Right. So you changed initFrameBuffer to not call
scroggo_chromium 2017/02/27 20:31:05 Yes.
Noel Gordon 2017/03/01 09:26:49 Acknowledged.
+ longjmp(JMPBUF(m_reader->pngPtr()), 1);
Noel Gordon 2017/02/21 13:53:54 Second issue here: initFrameBuffer() calls onInitF
scroggo_chromium 2017/02/23 22:09:52 Some options, suggested in above responses: - stop
Noel Gordon 2017/02/27 07:45:31 good
scroggo_chromium 2017/02/27 20:31:05 Sounds fine to me. Done in the latest patch set.
Noel Gordon 2017/03/01 09:26:49 Ok good. nits: use size.area() and move the color
scroggo_chromium 2017/03/03 21:56:40 I reverted back to the original code, since after
Noel Gordon 2017/02/21 13:53:55 Third issue here: diff-left calls buffer.setHasAlp
scroggo_chromium 2017/02/23 22:09:53 Alpha is tracked differently in the multi-frame co
Noel Gordon 2017/02/27 07:45:31 modulo the bug in the GIF code, this paragraph des
scroggo_chromium 2017/02/27 20:31:05 Could you be more specific? Once the frame is comp
Noel Gordon 2017/03/01 11:29:46 Yeap, sorry, I was replying to this part: "But th
scroggo_chromium 2017/03/03 21:56:40 Happy to provide a layout test. Is there a particu
Noel Gordon 2017/03/06 03:06:45 scroggo_chromium wrote:
Noel Gordon 2017/03/06 03:06:45 Yes, and I filed crbug.com/698487 about it, PTAL.
scroggo_chromium 2017/03/07 20:25:36 Filed https://bugs.chromium.org/p/chromium/issues/
Noel Gordon 2017/03/08 15:36:01 Yeap looked, good description of the issue therein
- buffer.setStatus(ImageFrame::FramePartial);
- buffer.setHasAlpha(false);
-
- // For PNGs, the frame always fills the entire image.
- buffer.setOriginalFrameRect(IntRect(IntPoint(), size()));
- }
+ ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+ const IntRect& frameRect = buffer.originalFrameRect();
Noel Gordon 2017/02/21 13:53:55 There is a nice DCHECK you used elsewhere to check
scroggo_chromium 2017/02/23 22:09:53 Done.
/* libpng comments (here to explain what follows).
- *
- * this function is called for every row in the image. If the
- * image is interlacing, and you turned on the interlace handler,
- * this function will be called for every row in every pass.
- * Some of these rows will not be changed from the previous pass.
- * When the row is not changed, the new_row variable will be NULL.
- * The rows and passes are called in order, so you don't really
- * need the row_num and pass, but I'm supplying them because it
- * may make your life easier.
- */
-
- // Nothing to do if the row is unchanged, or the row is outside
- // the image bounds: libpng may send extra rows, ignore them to
- // make our lives easier.
+ *
+ * this function is called for every row in the image. If the
+ * image is interlacing, and you turned on the interlace handler,
+ * this function will be called for every row in every pass.
+ * Some of these rows will not be changed from the previous pass.
+ * When the row is not changed, the new_row variable will be NULL.
+ * The rows and passes are called in order, so you don't really
+ * need the row_num and pass, but I'm supplying them because it
+ * may make your life easier.
+ */
+
+ // Nothing to do if the row is unchanged, or the row is outside the image
Noel Gordon 2017/02/21 13:53:55 // Nothing to do if the row is unchanged, or the r
scroggo_chromium 2017/02/23 22:09:54 Done.
+ // bounds. In the case that a frame presents more data than the indicated
+ // frame size, ignore the extra rows and thus use the frame size as the
+ // source of truth. libpng may also send extra rows. Ignore those as well.
+ // This prevents us from trying to write outside of the image bounds.
if (!rowBuffer)
return;
- int y = rowIndex;
- if (y < 0 || y >= size().height())
+ int y = rowIndex + frameRect.y();
+ DCHECK_GE(y, 0);
+ if (y >= size().height())
return;
/* libpng comments (continued).
- *
- * For the non-NULL rows of interlaced images, you must call
- * png_progressive_combine_row() passing in the row and the
- * old row. You can call this function for NULL rows (it will
- * just return) and for non-interlaced images (it just does the
- * memcpy for you) if it will make the code easier. Thus, you
- * can just do this for all cases:
- *
- * png_progressive_combine_row(png_ptr, old_row, new_row);
- *
- * where old_row is what was displayed for previous rows. Note
- * that the first pass (pass == 0 really) will completely cover
- * the old row, so the rows do not have to be initialized. After
- * the first pass (and only for interlaced images), you will have
- * to pass the current row, and the function will combine the
- * old row and the new row.
- */
-
- bool hasAlpha = m_reader->hasAlpha();
+ *
+ * For the non-NULL rows of interlaced images, you must call
+ * png_progressive_combine_row() passing in the row and the
+ * old row. You can call this function for NULL rows (it will
+ * just return) and for non-interlaced images (it just does the
+ * memcpy for you) if it will make the code easier. Thus, you
+ * can just do this for all cases:
+ *
+ * png_progressive_combine_row(png_ptr, old_row, new_row);
+ *
+ * where old_row is what was displayed for previous rows. Note
+ * that the first pass (pass == 0 really) will completely cover
+ * the old row, so the rows do not have to be initialized. After
+ * the first pass (and only for interlaced images), you will have
+ * to pass the current row, and the function will combine the
+ * old row and the new row.
+ */
+
+ bool hasAlpha = m_hasAlphaChannel;
png_bytep row = rowBuffer;
if (png_bytep interlaceBuffer = m_reader->interlaceBuffer()) {
@@ -300,9 +381,9 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
// Write the decoded row pixels to the frame buffer. The repetitive
// form of the row write loops is for speed.
- ImageFrame::PixelData* const dstRow = buffer.getAddr(0, y);
+ ImageFrame::PixelData* const dstRow = buffer.getAddr(frameRect.x(), y);
unsigned alphaMask = 255;
- int width = size().width();
+ int width = frameRect.width();
png_bytep srcPtr = row;
if (hasAlpha) {
@@ -324,20 +405,48 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
srcPtr = (png_bytep)dstRow;
Noel Gordon 2017/02/21 13:53:55 nit and not your doing, mind: C-style cast not all
scroggo_chromium 2017/02/23 22:09:53 Addressed in crrev.com/2716533002
}
- if (buffer.premultiplyAlpha()) {
- for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
- dstPixel++, srcPtr += 4) {
- buffer.setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
- srcPtr[3]);
- alphaMask &= srcPtr[3];
+ if (m_frameBufferCache[m_currentFrame].getAlphaBlendSource() ==
+ ImageFrame::BlendAtopBgcolor) {
+ if (buffer.premultiplyAlpha()) {
+ for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+ dstPixel++, srcPtr += 4) {
+ buffer.setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+ srcPtr[3]);
+ alphaMask &= srcPtr[3];
+ }
+ } else {
+ for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+ dstPixel++, srcPtr += 4) {
+ buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+ srcPtr[3]);
+ alphaMask &= srcPtr[3];
+ }
}
} else {
- for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
- dstPixel++, srcPtr += 4) {
- buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], srcPtr[3]);
- alphaMask &= srcPtr[3];
+ // Now, the blend method is ImageFrame::BlendAtopPreviousFrame. Since the
+ // frame data of the previous frame is copied at initFrameBuffer, we can
+ // blend the pixel of this frame, stored in |srcPtr|, over the previous
+ // pixel stored in |dstPixel|.
+ if (buffer.premultiplyAlpha()) {
+ for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+ dstPixel++, srcPtr += 4) {
+ buffer.blendRGBAPremultiplied(dstPixel, srcPtr[0], srcPtr[1],
+ srcPtr[2], srcPtr[3]);
+ alphaMask &= srcPtr[3];
+ }
+ } else {
+ for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
+ dstPixel++, srcPtr += 4) {
+ buffer.blendRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2],
+ srcPtr[3]);
+ alphaMask &= srcPtr[3];
+ }
}
}
+
+ if (alphaMask != 255 && !m_currentBufferSawAlpha)
+ m_currentBufferSawAlpha = true;
+
} else {
for (auto *dstPixel = dstRow; dstPixel < dstRow + width;
dstPixel++, srcPtr += 3) {
@@ -353,38 +462,49 @@ void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer,
}
}
- if (alphaMask != 255 && !buffer.hasAlpha())
- buffer.setHasAlpha(true);
-
buffer.setPixelsChanged(true);
}
-void PNGImageDecoder::complete() {
- if (m_frameBufferCache.isEmpty())
- return;
-
- m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
+bool PNGImageDecoder::frameIsCompleteAtIndex(size_t index) const {
+ if (!m_reader)
+ return false;
+
+ // For non-animated images, return whether the status of the frame is
+ // ImageFrame::FrameComplete with ImageDecoder::frameIsCompleteAtIndex.
+ // This matches the behavior of WEBPImageDecoder.
+ if (index == 0 && m_reader->parseCompleted() && m_reader->frameCount() == 1)
Noel Gordon 2017/02/21 13:53:55 Comparisons to 0: use !index. if (!index && m_rea
scroggo_chromium 2017/02/23 22:09:53 Good point. I see two ways this condition could re
Noel Gordon 2017/02/27 07:45:31 I expect that the following bool PNGImageDecoder:
scroggo_chromium 2017/02/27 20:31:05 This method was originally intended to mean "frame
+ return ImageDecoder::frameIsCompleteAtIndex(index);
+
+ // For first frames of animated images, PNGImageReader exposes whether it is
+ // fully received through firstFrameFullyReceived(). Non-first frames are
Noel Gordon 2017/02/21 13:53:55 Let's break this comment into two parts. // For
scroggo_chromium 2017/02/23 22:09:53 Done.
+ // reported by |m_reader| once it has parsed all data for that frame, so we
+ // can simply return if the index is below the reported frame count.
+ if (index == 0)
+ return m_reader->firstFrameFullyReceived();
+ return (index < m_reader->frameCount());
}
-inline bool isComplete(const PNGImageDecoder* decoder) {
- return decoder->frameIsCompleteAtIndex(0);
+float PNGImageDecoder::frameDurationAtIndex(size_t index) const {
+ return (index < m_frameBufferCache.size()
Noel Gordon 2017/02/21 13:53:55 This ternary if statement is unclear due to 80-col
scroggo_chromium 2017/02/23 22:09:52 Done.
+ ? m_frameBufferCache[index].duration()
+ : 0);
}
-void PNGImageDecoder::decode(bool onlySize) {
- if (failed())
+void PNGImageDecoder::complete() {
Noel Gordon 2017/02/21 13:53:54 complete -> frameComplete
scroggo_chromium 2017/02/23 22:09:53 Done.
+ if (m_currentFrame >= m_frameBufferCache.size())
return;
- if (!m_reader)
- m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset);
+ if (m_reader->interlaceBuffer())
+ m_reader->clearInterlaceBuffer();
- // If we couldn't decode the image but have received all the data, decoding
- // has failed.
- if (!m_reader->decode(*m_data, onlySize) && isAllDataReceived())
- setFailed();
+ ImageFrame& buffer = m_frameBufferCache[m_currentFrame];
+ if (buffer.getStatus() == ImageFrame::FrameEmpty)
+ longjmp(JMPBUF(m_reader->pngPtr()), 1);
+
+ buffer.setStatus(ImageFrame::FrameComplete);
Noel Gordon 2017/02/21 13:53:55 This latches the decoded image frame alpha into th
- // If decoding is done or failed, we don't need the PNGImageReader anymore.
- if (isComplete(this) || failed())
- m_reader.reset();
+ if (!m_currentBufferSawAlpha)
+ correctAlphaWhenFrameBufferSawNoAlpha(m_currentFrame);
Noel Gordon 2017/02/21 13:53:55 This code can change the image frame alpha state b
scroggo_chromium 2017/02/23 22:09:54 Done.
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698