New issue
Advanced search Search tips

Issue 701143 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 700892
issue 702148



Sign in to add a comment

Refactor ImageFrame::setSizeAndColorSpace

Project Member Reported by cblume@chromium.org, Mar 13 2017

Issue description

This function does multiple things -- it sets the size and color space & it zero fills the pixel data.

We want to be able to not zero fill the entire image. We will be filling the whole image with pixel data anyway. In the case of partial decodes we can manually zero fill only the portion that does not have image data.

Refactor ImageFrame::setSizeAndColorSpace to not also zero fill the pixel data. Callers should do that as a separate step if they need it.
 

Comment 1 by noel@chromium.org, Mar 16 2017

Blockedon: 700892

Comment 2 by noel@chromium.org, Mar 16 2017

Blockedon: 702148

Comment 3 by noel@chromium.org, Mar 16 2017

Came up during review, no test coverage for partially rendered animated GIF or APNG.  Added the those bugs here to fix that.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6b96c3de7c6db78762ce660b274514bd18a1c0b

commit b6b96c3de7c6db78762ce660b274514bd18a1c0b
Author: cblume <cblume@chromium.org>
Date: Tue Mar 21 16:43:05 2017

Refactor ImageFrame::setSizeAndColorSpace()

The function ImageFrame::setSizeAndColorSpace does multiple things.
It sets the size and color space & zero fills the data.

We want to be able to not zero fill the entire image. The image decoder
can fill the entire image frame with valid pixel data. And in the case
of partial images, the decoder knows which pixels do not contain image
data, allowing them to zero fill a smaller area.

But for now, just separate the concept of setSizeAndColorSpace() and
zeroFillPixelData().

No change in behavior, no new tests.

R=scroggo@chromium.org
BUG= 701143 

Review-Url: https://codereview.chromium.org/2749703002
Cr-Commit-Position: refs/heads/master@{#458437}

[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/graphics/test/MockImageDecoder.h
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/ImageFrame.h
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/jpeg/JPEGImageDecoder.cpp
[modify] https://crrev.com/b6b96c3de7c6db78762ce660b274514bd18a1c0b/third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp

Comment 5 by cblume@chromium.org, Mar 21 2017

Status: Fixed (was: Assigned)

Comment 6 by noel@chromium.org, Mar 23 2017

Blockedon: 704425

Comment 7 by noel@chromium.org, Mar 23 2017

Blockedon: -704425

Sign in to add a comment