This somewhat replaces the diff functionality which was removed. This adds two parameters to gm, ...
12 years, 5 months ago
(2013-01-11 16:41:49 UTC)
#1
This somewhat replaces the diff functionality which was removed. This adds two
parameters to gm, writeComparisonPicturePath and writeComparisonPath. These are
simply paths which, if specified, indicate where to write the .png and .skp
files from the comparison bitmaps and pictures. I found this ability to be very
helpful recently when trying to debug why the rtree picture and bitmap were
differing from the reference picture and bitmap.
Note that this also changes the make_filename a bit, changing pathSuffix to be
more like filenameSuffix. This makes more sense now that we're specifying the
paths directly.
I just noticed https://codereview.appspot.com/7100043/ which is rather similar in many ways. I had something like ...
12 years, 5 months ago
(2013-01-11 16:52:56 UTC)
#2
I just noticed https://codereview.appspot.com/7100043/ which is rather similar
in many ways. I had something like that at one point but locally found it to be
more instructive to debugging to just to output all of them (not just the
failing ones) as well as having a means of writing out the pictures to .skp
files.
https://codereview.appspot.com/7104043/diff/1/gm/gmmain.cpp File gm/gmmain.cpp (right): https://codereview.appspot.com/7104043/diff/1/gm/gmmain.cpp#newcode1 gm/gmmain.cpp:1: /* I see you have already noticed junov's partially ...
12 years, 5 months ago
(2013-01-11 16:59:22 UTC)
#3
On 2013/01/11 16:59:22, epoger wrote: > I think we should wait until those land (or ...
12 years, 5 months ago
(2013-01-11 17:14:03 UTC)
#4
On 2013/01/11 16:59:22, epoger wrote:
> I think we should wait until those land (or die), and then take another spin
at
> this one. Is that OK with you? Can you just use it locally for the next few
> days, in the meanwhile?
Well, you're the reviewer, so I don't have too much choice :-) I suppose this is
another use case to keep in mind with all the gmmain upheaval. What I'm looking
for is just a simple means of telling GM to write out all the bitmaps and
pictures it's comparing so I can look at them.
On 2013/01/11 17:14:03, bungeman wrote: > Well, you're the reviewer, so I don't have too ...
12 years, 5 months ago
(2013-01-11 17:50:20 UTC)
#5
On 2013/01/11 17:14:03, bungeman wrote:
> Well, you're the reviewer, so I don't have too much choice :-) I suppose this
is
> another use case to keep in mind with all the gmmain upheaval. What I'm
looking
> for is just a simple means of telling GM to write out all the bitmaps and
> pictures it's comparing so I can look at them.
You flatter me. I happen to know that you have commit power. :-)
If there is a significant need to get this change committed in the next few
days--as opposed to working in your local checkout--let me know, and we can
figure out the best way to merge the concurrent changes.
See https://codereview.appspot.com/7088044/diff/1/gm/gmmain.cpp#newcode467 ...
if/when that CL lands, it will be easy to start reporting results for ALL
"comparison" bitmaps. We should be able to do something similar for skps.
If we commit this change before that one, we'll have to redo most of the logic
anyway, so I think we're better off waiting if that's not too painful.
Ben- now that these two CLs have been committed: 1. https://code.google.com/p/skia/source/detail?r=7143 ('Adding commandline option to ...
12 years, 5 months ago
(2013-01-15 19:19:55 UTC)
#6
Ben- now that these two CLs have been committed:
1. https://code.google.com/p/skia/source/detail?r=7143 ('Adding commandline
option to gm to make it write image results for tests that fails due to pixel
mi…')
2. https://code.google.com/p/skia/source/detail?r=7198 ('gm: use new
Expectations struct instead of comparison bitmaps')
I think it's a good time for you to revisit this CL. The underlying code has
changed quite a bit, so I think your best bet will probably be to MANUALLY apply
your original patch to the latest code.
I will be in the office tomorrow, so I can stare at the code with you if that
will help.
In any event, I strongly encourage you to add coverage to the gm self-tests to
exercise your change. Otherwise, future changes may break it...
Seeing that everything is changing anyway, I'll just keep this change around in the darkness ...
12 years, 5 months ago
(2013-01-15 21:41:07 UTC)
#7
Message was sent while issue was closed.
Seeing that everything is changing anyway, I'll just keep this change around in
the darkness and make things work the way I want them to work again when I need
it.
Issue 7104043: Add ability to write out gm comparisons.
(Closed)
Created 12 years, 5 months ago by bungeman
Modified 12 years, 5 months ago
Reviewers: epoger
Base URL: http://skia.googlecode.com/svn/trunk/
Comments: 1