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

Issue 108963002: DM: Add support for reading a directory of images with --expectations (-r). (Closed)

Created:
7 years ago by mtklein
Modified:
7 years ago
Reviewers:
bsalomon, reed1
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Add support for reading a directory of images with --expectations (-r). DM writes out its images in a hierarchy that's a little different than GM, so this can't read GM's output. But it can read its own, written with -w. Example usage: $ out/Release/dm -w /tmp/baseline $ out/Release/dm -r /tmp/baseline -w /tmp/new (and optionally) $ mkdir /tmp/diff; out/Release/skdiff /tmp/baseline /tmp/new /tmp/diff GM's IndividualImageExpectationsSource and Expectations are a little too eager about decoding and hashing the expected images, so I took the opportunity to add DM::Expectations that mostly replaces skiagm::ExpectationsSource and skiagm::Expectations in DM. It mainly exists to move the image decoding and comparison off the main thread, which would otherwise be a major speed bottleneck. I tried to use skiagm code where possible. One notable place where I differed is in this new feature. When -r is a directory of images, DM does no hashing. It considerably faster to read the expected file into an SkBitmap and do a byte-for-byte comparison than to hash the two bitmaps and check those. The example usage above isn't quite working 100% yet. Expectations on some GMs fail, even with no binary change. I haven't pinned down whether this is due to - a bug in DM - flaky GMs - unthreadsafe GMs - flaky image decoding - unthreadsafe image decoding - something else but I intend to. Leon, Derek and I have suspected PNG decoding isn't threadsafe, but are as yet unable to prove it. I also seem to be able to cause malloc to fail on my laptop if I run too many configs at once, though I never seem to be using more than ~1G of RAM. Will track that down too. BUG= Committed: http://code.google.com/p/skia/source/detail?r=12596

Patch Set 1 #

Patch Set 2 : comments etc #

Unified diffs Side-by-side diffs Delta from patch set Stats (+173 lines, -112 lines) Patch
M dm/DM.cpp View 4 chunks +12 lines, -14 lines 0 comments Download
D dm/DMChecksumTask.h View 1 chunk +0 lines, -30 lines 0 comments Download
D dm/DMChecksumTask.cpp View 1 chunk +0 lines, -26 lines 0 comments Download
M dm/DMCpuTask.h View 4 chunks +3 lines, -5 lines 0 comments Download
M dm/DMCpuTask.cpp View 3 chunks +4 lines, -4 lines 0 comments Download
A dm/DMExpectations.h View 1 chunk +46 lines, -0 lines 0 comments Download
A + dm/DMExpectationsTask.h View 2 chunks +8 lines, -8 lines 0 comments Download
A dm/DMExpectationsTask.cpp View 1 chunk +21 lines, -0 lines 0 comments Download
M dm/DMGpuTask.h View 4 chunks +3 lines, -3 lines 0 comments Download
M dm/DMGpuTask.cpp View 4 chunks +4 lines, -4 lines 0 comments Download
M dm/DMUtil.h View 1 chunk +0 lines, -3 lines 0 comments Download
M dm/DMUtil.cpp View 1 chunk +0 lines, -5 lines 0 comments Download
M dm/DMWriteTask.h View 1 2 chunks +12 lines, -0 lines 0 comments Download
M dm/DMWriteTask.cpp View 1 4 chunks +59 lines, -9 lines 0 comments Download
M gyp/dm.gyp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
mtklein
7 years ago (2013-12-06 21:37:13 UTC) #1
mtklein
On 2013/12/06 21:37:13, mtklein wrote: Forgot to mention: on my laptop checking against images with ...
7 years ago (2013-12-06 21:42:13 UTC) #2
bsalomon
lgtm
7 years ago (2013-12-09 16:00:21 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/mtklein@google.com/108963002/20001
7 years ago (2013-12-10 14:45:22 UTC) #4
commit-bot: I haz the power
7 years ago (2013-12-10 14:53:25 UTC) #5
Message was sent while issue was closed.
Change committed as 12596

Powered by Google App Engine
This is Rietveld 408576698