Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 2762643004: Separate zeroing pixel data from setting alpha (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months ago by cblume
Modified:
1 month, 3 weeks ago
CC:
chromium-reviews, blink-reviews, kinuko+watch
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Separate zeroing pixel data from setting alpha Zeroing the pixel data should only affect values. Setting the alpha on or off should only effect behavior. For example, if I have an opaque image and I set the pixel values to 0 my image should appear black. But we currently also enable the alpha in this case which will make my image appear transparent. That forces me to reset the alpha back to disabled after zeroing the pixel data. BUG=703304

Patch Set 1 #

Total comments: 6

Patch Set 2 : Allocate pixels to their set alpha, remove out-dated comment #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -9 lines) Patch
M third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp View 1 2 chunks +8 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp View 1 2 chunks +1 line, -4 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Depends on Patchset:

Messages

Total messages: 20 (11 generated)
cblume
PTAL
7 months ago (2017-03-20 20:34:19 UTC) #4
Peter Kasting
LGTM
7 months ago (2017-03-20 20:43:53 UTC) #7
Noel Gordon
LGTM - with nits. https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp File third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp (right): https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp#newcode127 third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp:127: // setSize() calls eraseARGB(), which ...
7 months ago (2017-03-20 21:09:03 UTC) #8
cblume
https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp File third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp (right): https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp#newcode113 third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp:113: m_premultiplyAlpha ? kPremul_SkAlphaType : kUnpremul_SkAlphaType, I wonder if this ...
7 months ago (2017-03-20 22:40:28 UTC) #11
Peter Kasting
https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp File third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp (right): https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp#newcode127 third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp:127: // setSize() calls eraseARGB(), which resets the alpha flag, ...
7 months ago (2017-03-20 22:42:04 UTC) #12
cblume
https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp File third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp (right): https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp#newcode127 third_party/WebKit/Source/platform/image-decoders/bmp/BMPImageReader.cpp:127: // setSize() calls eraseARGB(), which resets the alpha flag, ...
7 months ago (2017-03-20 23:14:01 UTC) #15
Noel Gordon
https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp File third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp (right): https://codereview.chromium.org/2762643004/diff/1/third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp#newcode113 third_party/WebKit/Source/platform/image-decoders/ImageFrame.cpp:113: m_premultiplyAlpha ? kPremul_SkAlphaType : kUnpremul_SkAlphaType, On 2017/03/20 22:40:28, cblume ...
7 months ago (2017-03-22 23:44:55 UTC) #18
Peter Kasting
Ping. Rietveld is deprecated. I want to make sure my queue here is empty. What's ...
1 month, 3 weeks ago (2017-08-29 04:12:17 UTC) #19
cblume
1 month, 3 weeks ago (2017-08-29 07:19:58 UTC) #20
Message was sent while issue was closed.
On 2017/08/29 04:12:17, Peter Kasting wrote:
> Ping.  Rietveld is deprecated.  I want to make sure my queue here is empty. 
> What's the status here?

Marked as closed.
The purpose of this patch was really to be a step towards SkCodec acceptance,
which we agreed to adopt. So this patch isn't really needed.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa