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

[Fredrik Höglund]

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.

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.

Fredrik

> 
> 
> On Wed, Jun 8, 2016 at 2:28 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > Hm, that's odd. I guess the buffer usage doesn't get set properly? I
> > won't be able to look at this until tonight at the earliest, feel free
> > to revert the change in the meanwhile.
> >
> >   -ilia
> >
> > On Wed, Jun 8, 2016 at 1:25 PM, Brian Paul <brianp at vmware.com> wrote:
> >> Ilia, this patch causes a regression in the piglit
> >> arb_uniform_buffer_object-rendering-dsa test with llvmpipe (at least).
> >>
> >> I haven't debugged it at all.
> >>
> >> -Brian
> >>
> >> On 06/07/2016 08:29 PM, Ilia Mirkin wrote:
> >>>
> >>> Module: Mesa
> >>> Branch: master
> >>> Commit: 6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02
> >>> URL:
> >>> https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3D6e6fd911da8a1d9cd62fe0a8a4cc0fb7bdccfe02&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY&s=zn9XJ75wNk6pxn5KNFcWfLu7sXurLGGcWfIvVtSMwEo&e=
> >>>
> >>> Author: Ilia Mirkin <imirkin at alum.mit.edu>
> >>> Date:   Sat Jun  4 13:26:46 2016 -0400
> >>>
> >>> st/mesa: use buffer usage history to set dirty flags for revalidation
> >>>
> >>> We were previously unconditionally doing this for arrays and ubo's, and
> >>> ignoring texture/storage/atomic buffers. Instead use the usage history
> >>> to determine which atoms need to be revalidated.
> >>>
> >>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >>> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> >>> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> >>>
> >>> ---
> >>>
> >>>   src/mesa/state_tracker/st_cb_bufferobjects.c | 15 +++++++++++++--
> >>>   1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> index 8bbc2f0..1a8aea3 100644
> >>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
> >>> @@ -332,8 +332,19 @@ st_bufferobj_data(struct gl_context *ctx,
> >>>         }
> >>>      }
> >>>
> >>> -   /* BufferData may change an array or uniform buffer, need to update it
> >>> */
> >>> -   st->dirty.st |= ST_NEW_VERTEX_ARRAYS | ST_NEW_UNIFORM_BUFFER;
> >>> +   /* The current buffer may be bound, so we have to revalidate all atoms
> >>> that
> >>> +    * might be using it.
> >>> +    */
> >>> +   /* TODO: Add arrays to usage history */
> >>> +   st->dirty.st |= ST_NEW_VERTEX_ARRAYS;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_UNIFORM_BUFFER)
> >>> +      st->dirty.st |= ST_NEW_UNIFORM_BUFFER;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_SHADER_STORAGE_BUFFER)
> >>> +      st->dirty.st |= ST_NEW_STORAGE_BUFFER;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_TEXTURE_BUFFER)
> >>> +      st->dirty.st |= ST_NEW_SAMPLER_VIEWS | ST_NEW_IMAGE_UNITS;
> >>> +   if (st_obj->Base.UsageHistory & USAGE_ATOMIC_COUNTER_BUFFER)
> >>> +      st->dirty.st |= ST_NEW_ATOMIC_BUFFER;
> >>>
> >>>      return GL_TRUE;
> >>>   }
> >>>
> >>> _______________________________________________
> >>> mesa-commit mailing list
> >>> mesa-commit at lists.freedesktop.org
> >>>
> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=oZCXDAQxyzodq06r0fdxhJ0TSS2VMnftkgDNyPiJGKY&s=TUTsMchaJhWjCx5k6tptwVWWVHJs-zDtCtJVpPhM1pM&e=
> >>>
> >>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>