[Mesa-dev] Mesa (master): st/mesa: use buffer usage history to set dirty flags for revalidation

[Roland Scheidegger]

Am 10.06.2016 um 03:11 schrieb Ilia Mirkin:
> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund <fredrik at kde.org> wrote:
>>> On Wednesday 08 June 2016, Ilia Mirkin wrote:
>>>> Glancing at the code (I don't even have a piglit checkout here):
>>>>
>>>> static void
>>>> set_ubo_binding(struct gl_context *ctx, ...)
>>>> ...
>>>>    /* If this is a real buffer object, mark it has having been used
>>>>     * at some point as a UBO.
>>>>     */
>>>>    if (size >= 0)
>>>>       bufObj->UsageHistory |= USAGE_UNIFORM_BUFFER;
>>>>
>>>> That seems bogus - what if the current size is 0 (unallocated), the
>>>> buffer object gets bound to a UBO endpoint, and then someone goes in
>>>> and does glBufferData()? Same for set_ssbo_binding.
>>>>
>>>>   -ilia
>>>
>>> The test is greater than or equal to zero, so the UsageHistory should
>>> be set even when the buffer is unallocated.
>>
>> Right, duh.
>>
>>>
>>> But the piglit test doesn't bind the buffer as a uniform buffer before
>>> it allocates it.  It allocates the buffer first with glNamedBufferData(),
>>> and then binds it.  The UsageHistory is still set to the default value in
>>> the glNamedBufferData() call, since the buffer has never been bound
>>> at that point.  But the uniform buffer state should still be marked as
>>> dirty in the glBindBufferRange() call.  I think this failure suggests
>>> that that doesn't happen for some reason.
>>
>> I haven't looked in GREAT detail, but the test does pass on nv50,
>> nvc0, and softpipe. It only fails on llvmpipe.
>>
>> Brian, this might be out of my comfort area to figure out... Given
>> that it's working on the other drivers, that seems more likely to be a
>> failing of llvmpipe somehow.
> 
> Another observation is that the square sizes/shapes are all correct.
> However the colors are all of the first square (red). So it seems like
> some issue is happening for the fragment shader, but not vertex.
> 

I've looked at this briefly and I'm not sure who's to blame.
It goes something like this:
- we don't get any set_constant_buffer calls anymore when the contents
of the buffer change. I don't think that's ok, looks like a bug to me?
Granted it's still the same buffer, just modfying a bound one.

- when the contents are updated, it ends up in some transfer stuff as
expected, in the end llvmpipe_transfer_map(). This checks if the
resource is referenced in the current scene, if so it would flush -
however this is only done for textures and rts (because UBO contents are
indeed copied to the scene, not referenced, this is quite ok here, we
don't want to flush).

- on the next draw, we'd check the dirty bits - we never got
set_constant_buffer (which would set LP_NEW_CONSTANTS and in turn
LP_SETUP_NEW_CONSTANTS), and llvmpipe_transfer_map didn't do anything,
so we don't pick up the changed contents, and continue to use the old
copied ones.

We could check if buffers are referenced in the scene and set the
LP_NEW_CONSTANTS bit accordingly, but I'm not sure what the st is doing
is really ok? (For vs, it doesn't matter that we miss the update - as
the offsets etc. are all the same the vs will just pick up the changes
automatically as we don't copy anything since this stuff all runs
synchronously.)

Roland