Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1838603002:
Mark webps as sRGB (Closed)
DescriptionMark webps as sRGB
This may cause some diffs in Gold.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1838603002
Committed: https://skia.googlesource.com/skia/+/6e077e140a6b603192e2395ba0adac7b670b3f03
Patch Set 1 #
Total comments: 2
Patch Set 2 : Added comment #Patch Set 3 : Accept any profileType #
Total comments: 2
Messages
Total messages: 20 (10 generated)
Description was changed from ========== Mark webps as sRGB BUG=skia: ========== to ========== Mark webps as sRGB BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
msarett@google.com changed reviewers: + herb@google.com, scroggo@google.com
We don't pull ICC profiles from webps yet... but the spec says that if there no profile, we should assume the image is sRGB. So let's mark all webps as sRGB for now. Herb, this should increase the number of sRGB images that you see by a bit.
Description was changed from ========== Mark webps as sRGB BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Mark webps as sRGB This may cause some diffs in Gold. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
reed@google.com changed reviewers: + reed@google.com
https://codereview.chromium.org/1838603002/diff/1/src/codec/SkWebpCodec.cpp File src/codec/SkWebpCodec.cpp (right): https://codereview.chromium.org/1838603002/diff/1/src/codec/SkWebpCodec.cpp#n... src/codec/SkWebpCodec.cpp:254: SkWebpCodec::SkWebpCodec(const SkImageInfo& info, SkStream* stream) // The spec says an unmarked image IS srgb, so we return that space here.
lgtm
https://codereview.chromium.org/1838603002/diff/1/src/codec/SkWebpCodec.cpp File src/codec/SkWebpCodec.cpp (right): https://codereview.chromium.org/1838603002/diff/1/src/codec/SkWebpCodec.cpp#n... src/codec/SkWebpCodec.cpp:254: SkWebpCodec::SkWebpCodec(const SkImageInfo& info, SkStream* stream) On 2016/03/26 01:11:35, reed1 wrote: > // The spec says an unmarked image IS srgb, so we return that space here. Done.
The CQ bit was checked by msarett@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com Link to the patchset: https://codereview.chromium.org/1838603002/#ps20001 (title: "Added comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1838603002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1838603002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
The CQ bit was checked by msarett@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com Link to the patchset: https://codereview.chromium.org/1838603002/#ps40001 (title: "Accept any profileType")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1838603002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1838603002/40001
Message was sent while issue was closed.
Description was changed from ========== Mark webps as sRGB This may cause some diffs in Gold. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Mark webps as sRGB This may cause some diffs in Gold. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/6e077e140a6b603192e2395ba0adac7b670b3f03 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://skia.googlesource.com/skia/+/6e077e140a6b603192e2395ba0adac7b670b3f03
Message was sent while issue was closed.
https://codereview.chromium.org/1838603002/diff/40001/src/codec/SkWebpCodec.cpp File src/codec/SkWebpCodec.cpp (right): https://codereview.chromium.org/1838603002/diff/40001/src/codec/SkWebpCodec.c... src/codec/SkWebpCodec.cpp:90: // will treat the encoded data as linear regardless of what the client linear? I thought this CL's whole purpose was to treat them as sRGB?
Message was sent while issue was closed.
https://codereview.chromium.org/1838603002/diff/40001/src/codec/SkWebpCodec.cpp File src/codec/SkWebpCodec.cpp (right): https://codereview.chromium.org/1838603002/diff/40001/src/codec/SkWebpCodec.c... src/codec/SkWebpCodec.cpp:90: // will treat the encoded data as linear regardless of what the client On 2016/04/07 12:35:54, scroggo wrote: > linear? I thought this CL's whole purpose was to treat them as sRGB? Well, we want to mark them as sRGB. I think treating them as sRGB is still a work in progress - both on our end and with Herb's work. But I think it helps Herb to see them marked proerly as sRGB. We'll still do the same decode regardless of whether the client asks for sRGB or linear (which may be correct or incorrect, I'm not sure). The comment should probably say - "We will treat the encoded data the same way regardless of what the client requests." FWIW, I copied this comment from conversion_possible() SkCodecPriv.h. |
