Issue 5695047: GPU device preserves pixel values across read/write/read of unpremul pixel values

Issue 5695047: GPU device preserves pixel values across read/write/read of unpremul pixel values (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
12 years, 2 months ago by bsalomon

Modified:
12 years, 2 months ago

Reviewers:
TomH

Base URL:
http://skia.googlecode.com/svn/trunk/

Visibility:
Public.

More Reviews

Description

Make GPU device guarantee that when unpremul pixels values are read, written back, and read again, the two reads will produce identical results.

Patch Set 3 : cleanup, add test #

Patch Set 5 : Fix unsigned/signed warning #

Created: 12 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+394 lines, -45 lines)			Patch
M	gyp/tests.gyp	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
M	src/core/SkConfig8888.h	View	1 2	1 chunk	+6 lines, -0 lines	0 comments	Download
M	src/core/SkConfig8888.cpp	View	1 2	1 chunk	+24 lines, -0 lines	0 comments	Download
M	src/gpu/GrContext.cpp	View	1 2 3	3 chunks	+69 lines, -8 lines	0 comments	Download
M	src/gpu/GrGpu.h	View		1 chunk	+7 lines, -0 lines	0 comments	Download
M	src/gpu/gl/GrGLProgram.h	View	1 2	4 chunks	+22 lines, -12 lines	0 comments	Download
M	src/gpu/gl/GrGLProgram.cpp	View	1 2	3 chunks	+36 lines, -17 lines	0 comments	Download
M	src/gpu/gl/GrGpuGL.h	View	1 2	3 chunks	+12 lines, -0 lines	0 comments	Download
M	src/gpu/gl/GrGpuGL.cpp	View	1 2 3 4	2 chunks	+80 lines, -0 lines	0 comments	Download
M	src/gpu/gl/GrGpuGLShaders.cpp	View	1 2 3	5 chunks	+31 lines, -8 lines	0 comments	Download
A	tests/PremulAlphaRoundTripTest.cpp	View	1 2	1 chunk	+106 lines, -0 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

TomH

LGTM http://codereview.appspot.com/5695047/diff/3001/src/core/SkConfig8888.h File src/core/SkConfig8888.h (right): http://codereview.appspot.com/5695047/diff/3001/src/core/SkConfig8888.h#newcode29 src/core/SkConfig8888.h:29: uint32_t b); So this function translates pixels from ...

12 years, 2 months ago (2012-02-23 15:13:32 UTC) #2

bsalomon

12 years, 2 months ago (2012-02-23 15:40:17 UTC) #3

Closed with 3237.

http://codereview.appspot.com/5695047/diff/3001/src/core/SkConfig8888.h
File src/core/SkConfig8888.h (right):

http://codereview.appspot.com/5695047/diff/3001/src/core/SkConfig8888.h#newco...
src/core/SkConfig8888.h:29: uint32_t b);
On 2012/02/23 15:13:32, TomH wrote:
> So this function translates pixels from their current config into ARGB?

No, it packs the a, r, g, b channel values passed in into the specified config.
(added a comment)

http://codereview.appspot.com/5695047/diff/3001/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

http://codereview.appspot.com/5695047/diff/3001/src/gpu/GrContext.cpp#newcode...
src/gpu/GrContext.cpp:1794: SkCanvas::Config8888* config8888) {
On 2012/02/23 15:13:32, TomH wrote:
> What's the meaning of the return value?

Not all GrPixelConfigs can be represented as Config8888 (e.g. the 565 config).
Added a comment.

http://codereview.appspot.com/5695047/diff/3001/src/gpu/gl/GrGLProgram.h
File src/gpu/gl/GrGLProgram.h (right):

http://codereview.appspot.com/5695047/diff/3001/src/gpu/gl/GrGLProgram.h#newc...
src/gpu/gl/GrGLProgram.h:146: flags can be set.
On 2012/02/23 15:13:32, TomH wrote:
> nit: may

Done.

http://codereview.appspot.com/5695047/diff/3001/src/gpu/gl/GrGLProgram.h#newc...
src/gpu/gl/GrGLProgram.h:224: uint8_t fOutputConfig;      // cases to enum
OutputConfig
On 2012/02/23 15:13:32, TomH wrote:
> nit: casts

Done.

http://codereview.appspot.com/5695047/diff/3001/src/gpu/gl/GrGpuGL.cpp
File src/gpu/gl/GrGpuGL.cpp (right):

http://codereview.appspot.com/5695047/diff/3001/src/gpu/gl/GrGpuGL.cpp#newcod...
src/gpu/gl/GrGpuGL.cpp:316: // compared to textures.
On 2012/02/23 15:13:32, TomH wrote:
> nit: "compared to" is unclear. Do you mean "than on"?

Done.

Expand All Messages | Collapse All Messages