OLD | NEW |
---|---|
1 /* | 1 /* |
2 * Copyright (C) 2006 Apple Computer, Inc. | 2 * Copyright (C) 2006 Apple Computer, Inc. |
3 * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved. | 3 * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved. |
4 * | 4 * |
5 * Portions are Copyright (C) 2001 mozilla.org | 5 * Portions are Copyright (C) 2001 mozilla.org |
6 * | 6 * |
7 * Other contributors: | 7 * Other contributors: |
8 * Stuart Parmenter <stuart@mozilla.com> | 8 * Stuart Parmenter <stuart@mozilla.com> |
9 * | 9 * |
10 * This library is free software; you can redistribute it and/or | 10 * This library is free software; you can redistribute it and/or |
(...skipping 20 matching lines...) Expand all Loading... | |
31 * licenses (the MPL or the GPL) and not to allow others to use your | 31 * licenses (the MPL or the GPL) and not to allow others to use your |
32 * version of this file under the LGPL, indicate your decision by | 32 * version of this file under the LGPL, indicate your decision by |
33 * deletingthe provisions above and replace them with the notice and | 33 * deletingthe provisions above and replace them with the notice and |
34 * other provisions required by the MPL or the GPL, as the case may be. | 34 * other provisions required by the MPL or the GPL, as the case may be. |
35 * If you do not delete the provisions above, a recipient may use your | 35 * If you do not delete the provisions above, a recipient may use your |
36 * version of this file under any of the LGPL, the MPL or the GPL. | 36 * version of this file under any of the LGPL, the MPL or the GPL. |
37 */ | 37 */ |
38 | 38 |
39 #include "platform/image-decoders/png/PNGImageDecoder.h" | 39 #include "platform/image-decoders/png/PNGImageDecoder.h" |
40 | 40 |
41 #include "platform/image-decoders/png/PNGImageReader.h" | |
42 #include "png.h" | 41 #include "png.h" |
43 #include "wtf/PtrUtil.h" | |
44 #include <memory> | 42 #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.
| |
45 | 43 |
46 namespace blink { | 44 namespace blink { |
47 | 45 |
48 PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption, | 46 PNGImageDecoder::PNGImageDecoder(AlphaOption alphaOption, |
49 const ColorBehavior& colorBehavior, | 47 const ColorBehavior& colorBehavior, |
50 size_t maxDecodedBytes, | 48 size_t maxDecodedBytes, |
51 size_t offset) | 49 size_t offset) |
52 : ImageDecoder(alphaOption, colorBehavior, maxDecodedBytes), | 50 : ImageDecoder(alphaOption, colorBehavior, maxDecodedBytes), |
53 m_offset(offset) {} | 51 m_offset(offset), |
52 m_currentFrame(0), | |
53 // 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.
| |
54 // uses that as a signal to never check again, meaning the actual count | |
55 // will never be respected. | |
56 m_repetitionCount(cAnimationLoopOnce), | |
57 m_hasAlphaChannel(false), | |
58 m_currentBufferSawAlpha(false) {} | |
54 | 59 |
55 PNGImageDecoder::~PNGImageDecoder() {} | 60 PNGImageDecoder::~PNGImageDecoder() {} |
56 | 61 |
62 bool PNGImageDecoder::setFailed() { | |
63 m_reader.reset(); | |
64 return ImageDecoder::setFailed(); | |
65 } | |
66 | |
67 size_t PNGImageDecoder::decodeFrameCount() { | |
68 parse(PNGImageReader::PNGParseQuery::PNGMetaDataQuery); | |
Noel Gordon
2017/02/21 13:53:54
parse(ParseQuery::MetaData);
scroggo_chromium
2017/02/23 22:09:53
Done.
| |
69 return failed() ? m_frameBufferCache.size() : m_reader->frameCount(); | |
70 } | |
71 | |
72 void PNGImageDecoder::decode(size_t index) { | |
73 parse(PNGImageReader::PNGParseQuery::PNGMetaDataQuery); | |
Noel Gordon
2017/02/21 13:53:55
parse(ParseQuery::MetaData);
scroggo_chromium
2017/02/23 22:09:52
Done.
| |
74 | |
75 if (failed()) | |
76 return; | |
77 | |
78 updateAggressivePurging(index); | |
79 | |
80 Vector<size_t> framesToDecode = findFramesToDecode(index); | |
81 for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); i++) { | |
82 m_currentFrame = *i; | |
83 if (!m_reader->decode(*m_data, *i)) { | |
84 setFailed(); | |
85 return; | |
86 } | |
87 | |
88 // If this returns false, we need more data to continue decoding. | |
89 if (!postDecodeProcessing(*i)) | |
90 break; | |
91 } | |
92 | |
93 // It is also a fatal error if all data is received and we have decoded all | |
94 // frames available but the file is truncated. | |
95 if (index >= m_frameBufferCache.size() - 1 && isAllDataReceived() && | |
96 m_reader && !m_reader->parseCompleted()) | |
97 setFailed(); | |
98 } | |
99 | |
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
| |
100 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.
| |
101 if (m_reader) | |
102 m_reader->clearDecodeState(frameIndex); | |
103 ImageDecoder::clearFrameBuffer(frameIndex); | |
104 } | |
105 | |
106 bool PNGImageDecoder::onInitFrameBuffer(size_t index) { | |
107 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.
| |
108 png_get_interlace_type(m_reader->pngPtr(), m_reader->infoPtr())) { | |
109 unsigned colorChannels = m_hasAlphaChannel ? 4 : 3; | |
110 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.
| |
111 size().height()); | |
112 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.
| |
113 } | |
114 return true; | |
115 } | |
116 | |
117 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.
| |
118 DCHECK(frameIndex < m_frameBufferCache.size()); | |
119 return m_frameBufferCache[frameIndex].getDisposalMethod() != | |
120 ImageFrame::DisposeOverwritePrevious; | |
121 } | |
122 | |
123 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.
| |
124 if (failed() || (m_reader && m_reader->parseCompleted())) | |
125 return; | |
126 | |
127 if (!m_reader) | |
128 m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset); | |
129 | |
130 if (!m_reader->parse(*m_data, query)) | |
131 setFailed(); | |
132 } | |
133 | |
134 void PNGImageDecoder::setRepetitionCount(int repetitionCount) { | |
135 m_repetitionCount = repetitionCount; | |
136 } | |
137 | |
138 int PNGImageDecoder::repetitionCount() const { | |
139 return failed() ? cAnimationLoopOnce : m_repetitionCount; | |
140 } | |
141 | |
142 void PNGImageDecoder::initializeNewFrame(size_t index) { | |
143 const PNGImageReader::FrameInfo& frameInfo = m_reader->frameInfo(index); | |
144 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.
| |
145 | |
146 DCHECK(IntRect(IntPoint(), size()).contains(frameInfo.frameRect)); | |
147 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.
| |
148 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.
| |
149 buffer->setDisposalMethod(frameInfo.disposalMethod); | |
150 buffer->setAlphaBlendSource(frameInfo.alphaBlend); | |
151 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.
| |
152 findRequiredPreviousFrame(index, false)); | |
153 } | |
154 | |
57 inline float pngFixedToFloat(png_fixed_point x) { | 155 inline float pngFixedToFloat(png_fixed_point x) { |
58 return ((float)x) * 0.00001f; | 156 return ((float)x) * 0.00001f; |
59 } | 157 } |
60 | 158 |
61 inline sk_sp<SkColorSpace> readColorSpace(png_structp png, png_infop info) { | 159 inline sk_sp<SkColorSpace> readColorSpace(png_structp png, png_infop info) { |
62 if (png_get_valid(png, info, PNG_INFO_sRGB)) { | 160 if (png_get_valid(png, info, PNG_INFO_sRGB)) { |
63 return SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named); | 161 return SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named); |
64 } | 162 } |
65 | 163 |
66 png_charp name = nullptr; | 164 png_charp name = nullptr; |
(...skipping 37 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
104 // However, the current behavior matches Safari and Firefox. | 202 // However, the current behavior matches Safari and Firefox. |
105 } | 203 } |
106 } | 204 } |
107 | 205 |
108 return nullptr; | 206 return nullptr; |
109 } | 207 } |
110 | 208 |
111 void PNGImageDecoder::headerAvailable() { | 209 void PNGImageDecoder::headerAvailable() { |
112 png_structp png = m_reader->pngPtr(); | 210 png_structp png = m_reader->pngPtr(); |
113 png_infop info = m_reader->infoPtr(); | 211 png_infop info = m_reader->infoPtr(); |
114 png_uint_32 width = png_get_image_width(png, info); | |
115 png_uint_32 height = png_get_image_height(png, info); | |
116 | 212 |
117 // Protect against large PNGs. See http://bugzil.la/251381 for more details. | 213 png_uint_32 width, height; |
118 const unsigned long maxPNGSize = 1000000UL; | |
119 if (width > maxPNGSize || height > maxPNGSize) { | |
120 longjmp(JMPBUF(png), 1); | |
121 return; | |
122 } | |
123 | |
124 // Set the image size now that the image header is available. | |
125 if (!setSize(width, height)) { | |
126 longjmp(JMPBUF(png), 1); | |
127 return; | |
128 } | |
129 | |
130 int bitDepth, colorType, interlaceType, compressionType, filterType, channels; | 214 int bitDepth, colorType, interlaceType, compressionType, filterType, channels; |
131 png_get_IHDR(png, info, &width, &height, &bitDepth, &colorType, | 215 png_get_IHDR(png, info, &width, &height, &bitDepth, &colorType, |
132 &interlaceType, &compressionType, &filterType); | 216 &interlaceType, &compressionType, &filterType); |
133 | 217 |
134 // The options we set here match what Mozilla does. | 218 // The options we set here match what Mozilla does. |
135 | 219 |
136 // Expand to ensure we use 24-bit for RGB and 32-bit for RGBA. | 220 // Expand to ensure we use 24-bit for RGB and 32-bit for RGBA. |
137 if (colorType == PNG_COLOR_TYPE_PALETTE || | 221 if (colorType == PNG_COLOR_TYPE_PALETTE || |
138 (colorType == PNG_COLOR_TYPE_GRAY && bitDepth < 8)) | 222 (colorType == PNG_COLOR_TYPE_GRAY && bitDepth < 8)) |
139 png_set_expand(png); | 223 png_set_expand(png); |
140 | 224 |
141 png_bytep trns = 0; | 225 png_bytep trns = 0; |
142 int trnsCount = 0; | 226 int trnsCount = 0; |
Noel Gordon
2017/02/21 13:53:54
Are |trns| and |trnsCount| used anywhere?
scroggo_chromium
2017/02/23 22:09:52
For completeness, this has been addressed in crrev
| |
143 if (png_get_valid(png, info, PNG_INFO_tRNS)) { | 227 if (png_get_valid(png, info, PNG_INFO_tRNS)) { |
144 png_get_tRNS(png, info, &trns, &trnsCount, 0); | 228 png_get_tRNS(png, info, &trns, &trnsCount, 0); |
145 png_set_expand(png); | 229 png_set_expand(png); |
146 } | 230 } |
147 | 231 |
148 if (bitDepth == 16) | 232 if (bitDepth == 16) |
149 png_set_strip_16(png); | 233 png_set_strip_16(png); |
150 | 234 |
151 if (colorType == PNG_COLOR_TYPE_GRAY || | 235 if (colorType == PNG_COLOR_TYPE_GRAY || |
152 colorType == PNG_COLOR_TYPE_GRAY_ALPHA) | 236 colorType == PNG_COLOR_TYPE_GRAY_ALPHA) |
153 png_set_gray_to_rgb(png); | 237 png_set_gray_to_rgb(png); |
154 | 238 |
155 if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) { | 239 // 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.
| |
156 // We only support color profiles for color PALETTE and RGB[A] PNG. | 240 // frames also use this method, we don't want them to override the size of |
157 // Supporting color profiles for gray-scale images is slightly tricky, at | 241 // the image to the size of their frame rect. Frames also don't specify their |
158 // least using the CoreGraphics ICC library, because we expand gray-scale | 242 // own color space, so we need to set it only once. |
159 // images to RGB but we do not similarly transform the color profile. We'd | 243 if (!isDecodedSizeAvailable()) { |
160 // either need to transform the color profile or we'd need to decode into a | 244 // Protect against large PNGs. See http://bugzil.la/251381 for more details. |
161 // gray-scale image buffer and hand that to CoreGraphics. | 245 const unsigned long maxPNGSize = 1000000UL; |
162 sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info); | 246 if (width > maxPNGSize || height > maxPNGSize) { |
163 if (colorSpace) { | 247 longjmp(JMPBUF(png), 1); |
164 setEmbeddedColorSpace(colorSpace); | 248 return; |
249 } | |
250 | |
251 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.
| |
252 longjmp(JMPBUF(png), 1); | |
253 return; | |
254 } | |
255 | |
256 if ((colorType & PNG_COLOR_MASK_COLOR) && !ignoresColorSpace()) { | |
257 // We only support color profiles for color PALETTE and RGB[A] PNG. | |
258 // Supporting color profiles for gray-scale images is slightly tricky, at | |
259 // least using the CoreGraphics ICC library, because we expand gray-scale | |
260 // images to RGB but we do not similarly transform the color profile. We'd | |
261 // either need to transform the color profile or we'd need to decode into | |
262 // a | |
263 // gray-scale image buffer and hand that to CoreGraphics. | |
264 sk_sp<SkColorSpace> colorSpace = readColorSpace(png, info); | |
265 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
| |
266 setEmbeddedColorSpace(colorSpace); | |
267 } | |
165 } | 268 } |
166 } | 269 } |
167 | 270 |
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.
| |
168 if (!hasEmbeddedColorSpace()) { | 271 if (!hasEmbeddedColorSpace()) { |
169 // TODO (msarett): | 272 // TODO (msarett): |
170 // Applying the transfer function (gamma) should be handled by | 273 // Applying the transfer function (gamma) should be handled by |
171 // SkColorSpaceXform. Here we always convert to a transfer function that | 274 // SkColorSpaceXform. Here we always convert to a transfer function that |
172 // is a 2.2 exponential. This is a little strange given that the dst | 275 // is a 2.2 exponential. This is a little strange given that the dst |
173 // transfer function is not necessarily a 2.2 exponential. | 276 // transfer function is not necessarily a 2.2 exponential. |
174 // TODO (msarett): | 277 // TODO (msarett): |
Noel Gordon
2017/02/21 13:53:55
Comment lines 277-280: remove. Fixed long ago.
scroggo_chromium
2017/02/23 22:09:52
Acknowledged.
Noel Gordon
2017/02/27 07:45:31
And also addressed by crrev.com/2716533002
| |
175 // Often, PNGs that specify their transfer function with the gAMA tag will | 278 // Often, PNGs that specify their transfer function with the gAMA tag will |
176 // also specify their gamut with the cHRM tag. We should read this tag | 279 // also specify their gamut with the cHRM tag. We should read this tag |
177 // and do a full color space transformation if it is present. | 280 // and do a full color space transformation if it is present. |
178 const double inverseGamma = 0.45455; | 281 const double inverseGamma = 0.45455; |
179 const double defaultGamma = 2.2; | 282 const double defaultGamma = 2.2; |
180 double gamma; | 283 double gamma; |
181 if (!ignoresColorSpace() && png_get_gAMA(png, info, &gamma)) { | 284 if (!ignoresColorSpace() && png_get_gAMA(png, info, &gamma)) { |
182 const double maxGamma = 21474.83; | 285 const double maxGamma = 21474.83; |
183 if ((gamma <= 0.0) || (gamma > maxGamma)) { | 286 if ((gamma <= 0.0) || (gamma > maxGamma)) { |
184 gamma = inverseGamma; | 287 gamma = inverseGamma; |
185 png_set_gAMA(png, info, gamma); | 288 png_set_gAMA(png, info, gamma); |
186 } | 289 } |
187 png_set_gamma(png, defaultGamma, gamma); | 290 png_set_gamma(png, defaultGamma, gamma); |
188 } else { | 291 } else { |
189 png_set_gamma(png, defaultGamma, inverseGamma); | 292 png_set_gamma(png, defaultGamma, inverseGamma); |
190 } | 293 } |
191 } | 294 } |
192 | 295 |
193 // Tell libpng to send us rows for interlaced pngs. | 296 // Tell libpng to send us rows for interlaced pngs. |
194 if (interlaceType == PNG_INTERLACE_ADAM7) | 297 if (interlaceType == PNG_INTERLACE_ADAM7) |
195 png_set_interlace_handling(png); | 298 png_set_interlace_handling(png); |
196 | 299 |
197 // Update our info now. | 300 // 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.
| |
198 png_read_update_info(png, info); | 301 png_read_update_info(png, info); |
199 channels = png_get_channels(png, info); | 302 channels = png_get_channels(png, info); |
200 ASSERT(channels == 3 || channels == 4); | 303 DCHECK(channels == 3 || channels == 4); |
201 | 304 |
202 m_reader->setHasAlpha(channels == 4); | 305 m_hasAlphaChannel = (channels == 4); |
203 | 306 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.
| |
204 if (m_reader->decodingSizeOnly()) { | |
205 // If we only needed the size, halt the reader. | |
206 #if PNG_LIBPNG_VER_MAJOR > 1 || \ | |
207 (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5) | |
208 // Passing '0' tells png_process_data_pause() not to cache unprocessed data. | |
209 m_reader->setReadOffset(m_reader->currentBufferSize() - | |
210 png_process_data_pause(png, 0)); | |
211 #else | |
212 m_reader->setReadOffset(m_reader->currentBufferSize() - png->buffer_size); | |
213 png->buffer_size = 0; | |
214 #endif | |
215 } | |
216 } | 307 } |
217 | 308 |
218 void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer, | 309 void PNGImageDecoder::rowAvailable(unsigned char* rowBuffer, |
219 unsigned rowIndex, | 310 unsigned rowIndex, |
220 int) { | 311 int) { |
221 if (m_frameBufferCache.isEmpty()) | 312 if (m_currentFrame >= m_frameBufferCache.size()) |
222 return; | 313 return; |
223 | 314 |
224 // Initialize the framebuffer if needed. | 315 // When a client calls ImageDecoder::setMemoryAllocator *before* decoding the |
225 ImageFrame& buffer = m_frameBufferCache[0]; | 316 // frame count, the first frame won't get initialized correctly. The call will |
226 if (buffer.getStatus() == ImageFrame::FrameEmpty) { | 317 // resize |m_frameBufferCache| to 1, and therefore ImageDecoder::frameCount |
227 png_structp png = m_reader->pngPtr(); | 318 // will *not* call initializeNewFrame for the first frame, whether it is a |
228 if (!buffer.setSizeAndColorSpace(size().width(), size().height(), | 319 // static image or the first frame of an animated image. Amongst others, the |
229 colorSpaceForSkImages())) { | 320 // frame rect will not be set. If this is the case, initialize the frame here. |
230 longjmp(JMPBUF(png), 1); | 321 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
| |
231 return; | 322 initializeNewFrame(0); |
232 } | |
233 | 323 |
234 unsigned colorChannels = m_reader->hasAlpha() ? 4 : 3; | 324 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.
| |
235 if (PNG_INTERLACE_ADAM7 == | 325 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
| |
236 png_get_interlace_type(png, m_reader->infoPtr())) { | |
237 m_reader->createInterlaceBuffer(colorChannels * size().width() * | |
238 size().height()); | |
239 if (!m_reader->interlaceBuffer()) { | |
240 longjmp(JMPBUF(png), 1); | |
241 return; | |
242 } | |
243 } | |
244 | 326 |
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
| |
245 buffer.setStatus(ImageFrame::FramePartial); | 327 ImageFrame& buffer = m_frameBufferCache[m_currentFrame]; |
246 buffer.setHasAlpha(false); | 328 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.
| |
247 | |
248 // For PNGs, the frame always fills the entire image. | |
249 buffer.setOriginalFrameRect(IntRect(IntPoint(), size())); | |
250 } | |
251 | 329 |
252 /* libpng comments (here to explain what follows). | 330 /* libpng comments (here to explain what follows). |
253 * | 331 * |
254 * this function is called for every row in the image. If the | 332 * this function is called for every row in the image. If the |
255 * image is interlacing, and you turned on the interlace handler, | 333 * image is interlacing, and you turned on the interlace handler, |
256 * this function will be called for every row in every pass. | 334 * this function will be called for every row in every pass. |
257 * Some of these rows will not be changed from the previous pass. | 335 * Some of these rows will not be changed from the previous pass. |
258 * When the row is not changed, the new_row variable will be NULL. | 336 * When the row is not changed, the new_row variable will be NULL. |
259 * The rows and passes are called in order, so you don't really | 337 * The rows and passes are called in order, so you don't really |
260 * need the row_num and pass, but I'm supplying them because it | 338 * need the row_num and pass, but I'm supplying them because it |
261 * may make your life easier. | 339 * may make your life easier. |
262 */ | 340 */ |
263 | 341 |
264 // Nothing to do if the row is unchanged, or the row is outside | 342 // 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.
| |
265 // the image bounds: libpng may send extra rows, ignore them to | 343 // bounds. In the case that a frame presents more data than the indicated |
266 // make our lives easier. | 344 // frame size, ignore the extra rows and thus use the frame size as the |
345 // source of truth. libpng may also send extra rows. Ignore those as well. | |
346 // This prevents us from trying to write outside of the image bounds. | |
267 if (!rowBuffer) | 347 if (!rowBuffer) |
268 return; | 348 return; |
269 int y = rowIndex; | 349 int y = rowIndex + frameRect.y(); |
270 if (y < 0 || y >= size().height()) | 350 DCHECK_GE(y, 0); |
351 if (y >= size().height()) | |
271 return; | 352 return; |
272 | 353 |
273 /* libpng comments (continued). | 354 /* libpng comments (continued). |
274 * | 355 * |
275 * For the non-NULL rows of interlaced images, you must call | 356 * For the non-NULL rows of interlaced images, you must call |
276 * png_progressive_combine_row() passing in the row and the | 357 * png_progressive_combine_row() passing in the row and the |
277 * old row. You can call this function for NULL rows (it will | 358 * old row. You can call this function for NULL rows (it will |
278 * just return) and for non-interlaced images (it just does the | 359 * just return) and for non-interlaced images (it just does the |
279 * memcpy for you) if it will make the code easier. Thus, you | 360 * memcpy for you) if it will make the code easier. Thus, you |
280 * can just do this for all cases: | 361 * can just do this for all cases: |
281 * | 362 * |
282 * png_progressive_combine_row(png_ptr, old_row, new_row); | 363 * png_progressive_combine_row(png_ptr, old_row, new_row); |
283 * | 364 * |
284 * where old_row is what was displayed for previous rows. Note | 365 * where old_row is what was displayed for previous rows. Note |
285 * that the first pass (pass == 0 really) will completely cover | 366 * that the first pass (pass == 0 really) will completely cover |
286 * the old row, so the rows do not have to be initialized. After | 367 * the old row, so the rows do not have to be initialized. After |
287 * the first pass (and only for interlaced images), you will have | 368 * the first pass (and only for interlaced images), you will have |
288 * to pass the current row, and the function will combine the | 369 * to pass the current row, and the function will combine the |
289 * old row and the new row. | 370 * old row and the new row. |
290 */ | 371 */ |
291 | 372 |
292 bool hasAlpha = m_reader->hasAlpha(); | 373 bool hasAlpha = m_hasAlphaChannel; |
293 png_bytep row = rowBuffer; | 374 png_bytep row = rowBuffer; |
294 | 375 |
295 if (png_bytep interlaceBuffer = m_reader->interlaceBuffer()) { | 376 if (png_bytep interlaceBuffer = m_reader->interlaceBuffer()) { |
296 unsigned colorChannels = hasAlpha ? 4 : 3; | 377 unsigned colorChannels = hasAlpha ? 4 : 3; |
297 row = interlaceBuffer + (rowIndex * colorChannels * size().width()); | 378 row = interlaceBuffer + (rowIndex * colorChannels * size().width()); |
298 png_progressive_combine_row(m_reader->pngPtr(), row, rowBuffer); | 379 png_progressive_combine_row(m_reader->pngPtr(), row, rowBuffer); |
299 } | 380 } |
300 | 381 |
301 // Write the decoded row pixels to the frame buffer. The repetitive | 382 // Write the decoded row pixels to the frame buffer. The repetitive |
302 // form of the row write loops is for speed. | 383 // form of the row write loops is for speed. |
303 ImageFrame::PixelData* const dstRow = buffer.getAddr(0, y); | 384 ImageFrame::PixelData* const dstRow = buffer.getAddr(frameRect.x(), y); |
304 unsigned alphaMask = 255; | 385 unsigned alphaMask = 255; |
305 int width = size().width(); | 386 int width = frameRect.width(); |
306 | 387 |
307 png_bytep srcPtr = row; | 388 png_bytep srcPtr = row; |
308 if (hasAlpha) { | 389 if (hasAlpha) { |
309 // Here we apply the color space transformation to the dst space. | 390 // Here we apply the color space transformation to the dst space. |
310 // It does not really make sense to transform to a gamma-encoded | 391 // It does not really make sense to transform to a gamma-encoded |
311 // space and then immediately after, perform a linear premultiply. | 392 // space and then immediately after, perform a linear premultiply. |
312 // Ideally we would pass kPremul_SkAlphaType to xform->apply(), | 393 // Ideally we would pass kPremul_SkAlphaType to xform->apply(), |
313 // instructing SkColorSpaceXform to perform the linear premultiply | 394 // instructing SkColorSpaceXform to perform the linear premultiply |
314 // while the pixels are a linear space. | 395 // while the pixels are a linear space. |
315 // We cannot do this because when we apply the gamma encoding after | 396 // We cannot do this because when we apply the gamma encoding after |
316 // the premultiply, we will very likely end up with valid pixels | 397 // the premultiply, we will very likely end up with valid pixels |
317 // where R, G, and/or B are greater than A. The legacy drawing | 398 // where R, G, and/or B are greater than A. The legacy drawing |
318 // pipeline does not know how to handle this. | 399 // pipeline does not know how to handle this. |
319 if (SkColorSpaceXform* xform = colorTransform()) { | 400 if (SkColorSpaceXform* xform = colorTransform()) { |
320 SkColorSpaceXform::ColorFormat colorFormat = | 401 SkColorSpaceXform::ColorFormat colorFormat = |
321 SkColorSpaceXform::kRGBA_8888_ColorFormat; | 402 SkColorSpaceXform::kRGBA_8888_ColorFormat; |
322 xform->apply(colorFormat, dstRow, colorFormat, srcPtr, size().width(), | 403 xform->apply(colorFormat, dstRow, colorFormat, srcPtr, size().width(), |
323 kUnpremul_SkAlphaType); | 404 kUnpremul_SkAlphaType); |
324 srcPtr = (png_bytep)dstRow; | 405 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
| |
325 } | 406 } |
326 | 407 |
327 if (buffer.premultiplyAlpha()) { | 408 if (m_frameBufferCache[m_currentFrame].getAlphaBlendSource() == |
328 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | 409 ImageFrame::BlendAtopBgcolor) { |
329 dstPixel++, srcPtr += 4) { | 410 if (buffer.premultiplyAlpha()) { |
330 buffer.setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], | 411 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; |
331 srcPtr[3]); | 412 dstPixel++, srcPtr += 4) { |
332 alphaMask &= srcPtr[3]; | 413 buffer.setRGBAPremultiply(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], |
414 srcPtr[3]); | |
415 alphaMask &= srcPtr[3]; | |
416 } | |
417 } else { | |
418 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | |
419 dstPixel++, srcPtr += 4) { | |
420 buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], | |
421 srcPtr[3]); | |
422 alphaMask &= srcPtr[3]; | |
423 } | |
333 } | 424 } |
334 } else { | 425 } else { |
335 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | 426 // Now, the blend method is ImageFrame::BlendAtopPreviousFrame. Since the |
336 dstPixel++, srcPtr += 4) { | 427 // frame data of the previous frame is copied at initFrameBuffer, we can |
337 buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], srcPtr[3]); | 428 // blend the pixel of this frame, stored in |srcPtr|, over the previous |
338 alphaMask &= srcPtr[3]; | 429 // pixel stored in |dstPixel|. |
430 if (buffer.premultiplyAlpha()) { | |
431 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | |
432 dstPixel++, srcPtr += 4) { | |
433 buffer.blendRGBAPremultiplied(dstPixel, srcPtr[0], srcPtr[1], | |
434 srcPtr[2], srcPtr[3]); | |
435 alphaMask &= srcPtr[3]; | |
436 } | |
437 } else { | |
438 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | |
439 dstPixel++, srcPtr += 4) { | |
440 buffer.blendRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], | |
441 srcPtr[3]); | |
442 alphaMask &= srcPtr[3]; | |
443 } | |
339 } | 444 } |
340 } | 445 } |
446 | |
447 if (alphaMask != 255 && !m_currentBufferSawAlpha) | |
448 m_currentBufferSawAlpha = true; | |
449 | |
341 } else { | 450 } else { |
342 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; | 451 for (auto *dstPixel = dstRow; dstPixel < dstRow + width; |
343 dstPixel++, srcPtr += 3) { | 452 dstPixel++, srcPtr += 3) { |
344 buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], 255); | 453 buffer.setRGBARaw(dstPixel, srcPtr[0], srcPtr[1], srcPtr[2], 255); |
345 } | 454 } |
346 | 455 |
347 // We'll apply the color space xform to opaque pixels after they have been | 456 // We'll apply the color space xform to opaque pixels after they have been |
348 // written to the ImageFrame, purely because SkColorSpaceXform supports | 457 // written to the ImageFrame, purely because SkColorSpaceXform supports |
349 // RGBA (and not RGB). | 458 // RGBA (and not RGB). |
350 if (SkColorSpaceXform* xform = colorTransform()) { | 459 if (SkColorSpaceXform* xform = colorTransform()) { |
351 xform->apply(xformColorFormat(), dstRow, xformColorFormat(), dstRow, | 460 xform->apply(xformColorFormat(), dstRow, xformColorFormat(), dstRow, |
352 size().width(), kOpaque_SkAlphaType); | 461 size().width(), kOpaque_SkAlphaType); |
353 } | 462 } |
354 } | 463 } |
355 | 464 |
356 if (alphaMask != 255 && !buffer.hasAlpha()) | |
357 buffer.setHasAlpha(true); | |
358 | |
359 buffer.setPixelsChanged(true); | 465 buffer.setPixelsChanged(true); |
360 } | 466 } |
361 | 467 |
468 bool PNGImageDecoder::frameIsCompleteAtIndex(size_t index) const { | |
469 if (!m_reader) | |
470 return false; | |
471 | |
472 // For non-animated images, return whether the status of the frame is | |
473 // ImageFrame::FrameComplete with ImageDecoder::frameIsCompleteAtIndex. | |
474 // This matches the behavior of WEBPImageDecoder. | |
475 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
| |
476 return ImageDecoder::frameIsCompleteAtIndex(index); | |
477 | |
478 // For first frames of animated images, PNGImageReader exposes whether it is | |
479 // 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.
| |
480 // reported by |m_reader| once it has parsed all data for that frame, so we | |
481 // can simply return if the index is below the reported frame count. | |
482 if (index == 0) | |
483 return m_reader->firstFrameFullyReceived(); | |
484 return (index < m_reader->frameCount()); | |
485 } | |
486 | |
487 float PNGImageDecoder::frameDurationAtIndex(size_t index) const { | |
488 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.
| |
489 ? m_frameBufferCache[index].duration() | |
490 : 0); | |
491 } | |
492 | |
362 void PNGImageDecoder::complete() { | 493 void PNGImageDecoder::complete() { |
Noel Gordon
2017/02/21 13:53:54
complete -> frameComplete
scroggo_chromium
2017/02/23 22:09:53
Done.
| |
363 if (m_frameBufferCache.isEmpty()) | 494 if (m_currentFrame >= m_frameBufferCache.size()) |
364 return; | 495 return; |
365 | 496 |
366 m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete); | 497 if (m_reader->interlaceBuffer()) |
367 } | 498 m_reader->clearInterlaceBuffer(); |
368 | 499 |
369 inline bool isComplete(const PNGImageDecoder* decoder) { | 500 ImageFrame& buffer = m_frameBufferCache[m_currentFrame]; |
370 return decoder->frameIsCompleteAtIndex(0); | 501 if (buffer.getStatus() == ImageFrame::FrameEmpty) |
371 } | 502 longjmp(JMPBUF(m_reader->pngPtr()), 1); |
372 | 503 |
373 void PNGImageDecoder::decode(bool onlySize) { | 504 buffer.setStatus(ImageFrame::FrameComplete); |
Noel Gordon
2017/02/21 13:53:55
This latches the decoded image frame alpha into th
| |
374 if (failed()) | |
375 return; | |
376 | 505 |
377 if (!m_reader) | 506 if (!m_currentBufferSawAlpha) |
378 m_reader = WTF::makeUnique<PNGImageReader>(this, m_offset); | 507 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.
| |
379 | |
380 // If we couldn't decode the image but have received all the data, decoding | |
381 // has failed. | |
382 if (!m_reader->decode(*m_data, onlySize) && isAllDataReceived()) | |
383 setFailed(); | |
384 | |
385 // If decoding is done or failed, we don't need the PNGImageReader anymore. | |
386 if (isComplete(this) || failed()) | |
387 m_reader.reset(); | |
388 } | 508 } |
389 | 509 |
390 } // namespace blink | 510 } // namespace blink |
OLD | NEW |