> On Jul 24, 2017, at 10:22 AM, Roland Scheidegger <srol...@vmware.com> wrote:
>
> Am 24.07.2017 um 15:26 schrieb Jose Fonseca:
>> In particular:
>>
>> 1) For buffers the box expresses bytes, not pixels.
> This shouldn't be an issue, since buffers are required to have a 1-byte
> format (usually R8_UINT, but R8_UNORM works too).
> (llvmpipe_transfer_map, for instance, will still do all the format dance
> for calculating offsets, makes no difference either way.)
>
>
>>
>> 2) For textures we must not approximate the calculation with `stride *
>> height`, or `slice_stride * depth`, as that can easily lead to buffer
>> overflows, particularly for partial transfers.
>>
>> But this code path is not currently very relevant since the trace
>> driver is only dumping data for buffers.
>>
>> This should address the issue that Bruce Cherniak found and diagnosed.
Very much appreciated.
>
>> ---
>> src/gallium/drivers/trace/tr_dump.c | 30 +++++++++++++++++-------------
>> 1 file changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/gallium/drivers/trace/tr_dump.c
>> b/src/gallium/drivers/trace/tr_dump.c
>> index 78c72492dc..aab55644f3 100644
>> --- a/src/gallium/drivers/trace/tr_dump.c
>> +++ b/src/gallium/drivers/trace/tr_dump.c
>> @@ -450,21 +450,25 @@ void trace_dump_box_bytes(const void *data,
>> {
>> size_t size;
>>
>> - /*
>> - * Only dump buffer transfers to avoid huge files.
>> - * TODO: Make this run-time configurable
>> - */
>> - if (resource->target != PIPE_BUFFER) {
>> - size = 0;
>> + if (resource->target == PIPE_BUFFER) {
>> + /* For buffers the box should be in bytes, regardless of the format.
>> + */
> So, maybe an assert that format is actually a 1-byte format would make
> more sense instead.
>
> But otherwise, looks great.
> Reviewed-by: Roland Scheidegger <srol...@vmware.com>
>
>> + assert(box->height == 1);
>> + assert(box->depth == 1);
>> + size = box->width;
>> } else {
>> enum pipe_format format = resource->format;
>> - if (slice_stride)
>> - size = box->depth * slice_stride;
>> - else if (stride)
>> - size = util_format_get_nblocksy(format, box->height) * stride;
>> - else {
>> - size = util_format_get_nblocksx(format, box->width) *
>> util_format_get_blocksize(format);
>> - }
>> + assert(box->height > 0);
>> + assert(box->depth > 0);
>> + 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;
>> +
>> + /*
>> + * Only dump buffer transfers to avoid huge files.
>> + * TODO: Make this run-time configurable
>> + */
>> + size = 0;
>> }
>>
>> trace_dump_bytes(data, size);
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
