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

Issue 6877024: Add gamma support to ply-image. (Closed)

Created:
8 years, 4 months ago by marcheu
Modified:
8 years, 3 months ago
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

Add gamma support to ply-image. BUG=chromium-os:10138 TEST=Run chrome os with a color profile, check that it gets loaded (I use a profile with one of the components zeroed for testing so changes are obvious). Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=372eb64

Patch Set 1 #

Total comments: 22

Patch Set 2 : Fix style. #

Patch Set 3 : Add a command line option for gamma. #

Total comments: 1

Patch Set 4 : git cl push #

Unified diffs Side-by-side diffs Delta from patch set Stats (+167 lines, -2 lines) Patch
M src/Makefile View 2 chunks +3 lines, -1 line 0 comments Download
A src/ply-frame-buffer.c View 0 chunks +-1 lines, --1 lines 0 comments Download
A src/ply-gamma.h View 1 2 1 chunk +32 lines, -0 lines 0 comments Download
A src/ply-gamma.c View 1 2 1 chunk +114 lines, -0 lines 0 comments Download
M src/ply-image.c View 1 2 3 5 chunks +19 lines, -2 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
marcheu
This does nothing ATM (it won't touch the gamma ramp if it doesn't find the ...
8 years, 4 months ago (2011-04-18 23:46:04 UTC) #1
Luigi Semenzato
Looking at it now. On Mon, Apr 18, 2011 at 4:46 PM, <marcheu@chromium.org> wrote: > ...
8 years, 4 months ago (2011-04-19 00:40:31 UTC) #2
Luigi Semenzato
LGTM assuming that ply-gamma.c was taken "as is" from the ply-image distro. (Perhaps the review ...
8 years, 4 months ago (2011-04-19 00:57:26 UTC) #3
marcheu
On 2011/04/19 00:57:26, Luigi Semenzato wrote: > LGTM assuming that ply-gamma.c was taken "as is" ...
8 years, 4 months ago (2011-04-19 01:14:34 UTC) #4
Daniel Erat
LGTM after fixing a bunch of style things. http://codereview.chromium.org/6877024/diff/1/src/ply-gamma.c File src/ply-gamma.c (right): http://codereview.chromium.org/6877024/diff/1/src/ply-gamma.c#newcode27 src/ply-gamma.c:27: #include ...
8 years, 4 months ago (2011-04-19 16:46:02 UTC) #5
Luigi Semenzato
Actually, aren't we supposed to use the existing style when adding code to a program? ...
8 years, 4 months ago (2011-04-19 17:30:30 UTC) #6
Daniel Erat
Stephane asked me about this. My take is that the style in the existing files ...
8 years, 4 months ago (2011-04-19 17:35:42 UTC) #7
Luigi Semenzato
OK, it makes sense. It's not like we're ever merging with that code again. On ...
8 years, 4 months ago (2011-04-19 17:38:25 UTC) #8
marcheu
http://codereview.chromium.org/6877024/diff/1/src/ply-gamma.c File src/ply-gamma.c (right): http://codereview.chromium.org/6877024/diff/1/src/ply-gamma.c#newcode27 src/ply-gamma.c:27: #include "ply-gamma.h" On 2011/04/19 16:46:02, Daniel Erat wrote: > ...
8 years, 4 months ago (2011-04-19 18:02:45 UTC) #9
marcheu
Added gamma command line option, PTAL.
8 years, 4 months ago (2011-04-19 19:24:26 UTC) #10
Daniel Erat
LooksDisgustingToMe, but not your fault. Ship it!
8 years, 4 months ago (2011-04-19 20:10:27 UTC) #11
Daniel Erat
8 years, 4 months ago (2011-04-19 20:13:36 UTC) #12
http://codereview.chromium.org/6877024/diff/2005/src/ply-image.c
File src/ply-image.c (right):

http://codereview.chromium.org/6877024/diff/2005/src/ply-image.c#newcode521
src/ply-image.c:521: "       ply-image <background> [<x-offset> <y-offset> "
forgot to mention: add something about --gamma here

Powered by Google App Engine
This is Rietveld 408576698