[PATCH] retrace: support for dumping multiple snapshots (v2)
Rob Clark
robdclark at gmail.com
Wed Jan 25 00:22:20 UTC 2017
On Tue, Jan 24, 2017 at 11:54 AM, José Fonseca <jose.r.fonseca at gmail.com> wrote:
> On Sat, Jan 21, 2017 at 6:30 PM, Eric Anholt <eric at anholt.net> wrote:
>>
>> Rob Clark <robdclark at gmail.com> writes:
>>
>> > Usually if an app is using MRT, we want to dump (and diff) *all* the
>> > render targets to track down where things are going wrong. Also dumps
>> > depth and stencil buffers.
>> >
>> > Only implemented for GLDumper, since I don't know anything about D3D.
>> >
>> > TODO maybe cmdline arg to control this?
>>
>> I'm not sure, I kinda tend to
>> > think if you are using dump-images + diff-images you probably want to
>> > compare all the render targets.
>
>
> Yes, let's add a command line please. I fear that reading back
> depth-stencil might cause instability / and slow down things tremendously,
> as it can really be an heavy operation for HW which stores Z buffer in a
> non-linear fashion.
>
hmm, reading back color buffer(s) is already a pretty heavy operation
(ie. stalling the gpu)..
>>
>> > ---
>> > retrace/d3dretrace.hpp | 9 ++++++--
>> > retrace/glretrace_main.cpp | 12 +++++++++--
>> > retrace/glstate.hpp | 5 ++++-
>> > retrace/glstate_images.cpp | 51
>> > +++++++++++++++++++++++++++++++++++++---------
>> > retrace/retrace.hpp | 5 ++++-
>> > retrace/retrace_main.cpp | 50
>> > ++++++++++++++++++++++++++++++++++++---------
>> > 6 files changed, 106 insertions(+), 26 deletions(-)
>> >
>>
>> > @@ -200,9 +206,24 @@ takeSnapshot(unsigned call_no) {
>> > break;
>> > }
>> > } else {
>> > - os::String filename = os::String::format("%s%010u.png",
>> > - snapshotPrefix,
>> > - useCallNos ?
>> > call_no : snapshot_no);
>> > + os::String filename;
>> > +
>> > + if (mrt == -2) {
>> > + /* stencil */
>> > + filename = os::String::format("%s%010u-s.png",
>> > + snapshotPrefix,
>> > + useCallNos ? call_no :
>> > snapshot_no);
>> > + } else if (mrt == -1) {
>> > + /* depth */
>> > + filename = os::String::format("%s%010u-z.png",
>> > + snapshotPrefix,
>> > + useCallNos ? call_no :
>> > snapshot_no);
>> > + } else {
>> > + filename = os::String::format("%s%010u-mrt%u.png",
>> > + snapshotPrefix,
>> > + useCallNos ? call_no :
>> > snapshot_no,
>> > + mrt);
>> > + }
>>
>> The rename here is going to break piglit's testing of color rendering
>> output. Could we keep the old filename for the first color buffer, or
>> could you fix piglit to look for both?
>
>
> Yes, please let's keep the old filename for first color buffers. There are
> lots of test harnesses (besides piglit) that are built upon apitrace that
> would need to change if not.
well, if I'm going to add a cmdline option, perhaps old file name if
cmdline flag not set, new otherwise? Maybe it's my OCD but the sort
order is funny otherwise, ie.
[robclark at thunkpad:~/diff-images/vdrift/tmp]$ ls -l
total 792
-rw-rw-r--. 1 robclark robclark 171718 Jan 24 19:19 0000154202-mrt1.png
-rw-rw-r--. 1 robclark robclark 616406 Jan 24 19:19 0000154202-mrt2.png
-rw-rw-r--. 1 robclark robclark 6362 Jan 24 19:19 0000154202.png
-rw-rw-r--. 1 robclark robclark 11917 Jan 24 19:19 0000154202-z.png
;-)
BR,
-R
>>
>>
>> The patch (other than that) seems really useful, though.
>>
>> _______________________________________________
>> apitrace mailing list
>> apitrace at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/apitrace
>>
>
> Indeed. Thanks.
>
> Jose
More information about the apitrace
mailing list