Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 1304443002: Have DM manually encode its .png outputs. (Closed)

Created:
5 years, 4 months ago by mtklein_C
Modified:
5 years, 4 months ago
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

Have DM manually encode its .png outputs. This eliminates some variability on various axes: different PNG encoders, different libpng versions, different formats (RGB, indexed), different unpremultiplication, different sRGB tags. BUG=skia: Committed: https://skia.googlesource.com/skia/+/3cc0dfffb70c0bd08ed8899efcd2e98da86a6ec7 CQ_EXTRA_TRYBOTS=client.skia:Test-Win8-MSVC-ShuttleB-CPU-AVX2-x86_64-Debug-Trybot Committed: https://skia.googlesource.com/skia/+/a5114d7f26524f75b96e63ffd796d44749b6248c

Patch Set 1 #

Patch Set 2 : embed author and md5 #

Patch Set 3 : Add debug lines for failures. #

Patch Set 4 : Fixed? #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -34 lines) Patch
M dm/DM.cpp View 1 2 3 3 chunks +76 lines, -16 lines 1 comment Download
M gyp/dm.gypi View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M gyp/libpng.gyp View 1 2 2 chunks +21 lines, -18 lines 0 comments Download

Messages

Total messages: 28 (11 generated)
mtklein_C
5 years, 4 months ago (2015-08-18 18:31:21 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304443002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304443002/20001
5 years, 4 months ago (2015-08-18 18:31:34 UTC) #4
msarett
This looks great to me! lgtm
5 years, 4 months ago (2015-08-18 18:42:56 UTC) #5
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-18 18:42:58 UTC) #7
stephana
On 2015/08/18 18:42:58, commit-bot: I haz the power wrote: > Dry run: This issue passed ...
5 years, 4 months ago (2015-08-18 19:46:34 UTC) #8
mtklein
On 2015/08/18 19:46:34, stephana wrote: > On 2015/08/18 18:42:58, commit-bot: I haz the power wrote: ...
5 years, 4 months ago (2015-08-18 19:48:36 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304443002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304443002/20001
5 years, 4 months ago (2015-08-19 14:37:13 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/3cc0dfffb70c0bd08ed8899efcd2e98da86a6ec7
5 years, 4 months ago (2015-08-19 14:37:53 UTC) #12
mtklein_C
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1297383002/ by mtklein@chromium.org. ...
5 years, 4 months ago (2015-08-19 15:11:20 UTC) #13
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304443002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304443002/40001
5 years, 4 months ago (2015-08-19 19:36:26 UTC) #15
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: Test-Win8-MSVC-ShuttleB-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Win8-MSVC-ShuttleB-CPU-AVX2-x86_64-Debug-Trybot/builds/2)
5 years, 4 months ago (2015-08-19 20:03:57 UTC) #17
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304443002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304443002/60001
5 years, 4 months ago (2015-08-19 20:47:11 UTC) #19
mtklein_C
If the dry run passes, this is probably good to go. I'm going to be ...
5 years, 4 months ago (2015-08-19 20:55:00 UTC) #20
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 4 months ago (2015-08-19 21:09:42 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1304443002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1304443002/60001
5 years, 4 months ago (2015-08-24 20:06:07 UTC) #25
commit-bot: I haz the power
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/a5114d7f26524f75b96e63ffd796d44749b6248c
5 years, 4 months ago (2015-08-24 20:27:06 UTC) #26
hal.canary
5 years, 4 months ago (2015-08-24 20:35:06 UTC) #28
Message was sent while issue was closed.
https://codereview.chromium.org/1304443002/diff/60001/dm/DM.cpp
File dm/DM.cpp (right):

https://codereview.chromium.org/1304443002/diff/60001/dm/DM.cpp#newcode496
dm/DM.cpp:496: bitmap = SkBitmap();
bitmap.reset();

Powered by Google App Engine
This is Rietveld 408576698