[Mesa-dev] gallium: Reduce trace_dump_box_bytes size by box->x.

[Jose Fonseca]

Hi Bruce,


Sorry for the delay.  I finally had a bit of time to take a look at your original patch on https://lists.freedesktop.org/archives/mesa-dev/2017-February/142817.html , and it's definitely an improvement.


But a more straightforward way to do this is to, rather than adjusting for x,y,z after the fact, simply calculate the strict blob size, as in:


      size =  util_format_get_nblocksx(format, box->width )      * util_format_get_blocksize(format)
           + (util_format_get_nblocksy(format, box->height) - 1) * stride
           +                                  (box->depth   - 1) * slice_stride;


This should get the right results regardless any of x,y,z or stride or slice_stride is zero or not.  It does require that height & depth to never zero, but I think that's always the case already.


I noticed something even worse: we were not considering buffer transfer box as bytes.  Fixing this would also fix the issue I belive.


I posted a patch with the above for review.


Jose


________________________________
From: Cherniak, Bruce <bruce.cherniak at intel.com>
Sent: Wednesday, July 5, 2017 16:38
To: Jose Fonseca
Cc: Michel Dänzer; Marek Olšák; ML mesa-dev
Subject: Re: [Mesa-dev] gallium: Reduce trace_dump_box_bytes size by box->x.


On Jul 4, 2017, at 9:40 AM, Jose Fonseca <jfonseca at vmware.com<mailto:jfonseca at vmware.com>> wrote:

On 29/06/17 11:02, Michel Dänzer wrote:
On 29/06/17 01:54 AM, Cherniak, Bruce wrote:
On Jun 26, 2017, at 2:10 PM, Marek Olšák <maraeo at gmail.com<mailto:maraeo at gmail.com>> wrote:

In my opinion, dumping resources isn't very useful. I think it would
be better to remove that completely.

>From Michel's response, sounds like dumping resources is useful, so...
FWIW, after my previous post it occurred to me that resource dumping
might not be necessary after all; dumping transfers might be sufficient
for a replayable trace.
Adding José, I hope he can clarify.

Right, to preserve ability to replay, it should suffice to serialize the write transfers.

Of course, this ignores the issue of persistent mappings.  To handle persistent mappings the trace driver would need to, on every draw, walk over every bound buffer and see if any was changed by keeping a shadow memory, or some other trick mentioned on https://github.com/apitrace/apitrace/issues/232

With regards to averting the crash that I experience in trace_dump_box_bytes when tracing certain applications, what is the best path forward?
I can keep my patch (or a patch that stubs this function altogether) handy for local application, but that doesn't help anyone else out.

Thanks,
Bruce

Jose

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170724/30c6e3c9/attachment-0001.html>