On Thu, Jun 9, 2016 at 10:58 PM, Roland Scheidegger <srol...@vmware.com> wrote: > Am 10.06.2016 um 03:11 schrieb Ilia Mirkin: >> On Thu, Jun 9, 2016 at 9:07 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: >>> On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund <fred...@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.
That's very much intentional, at least as I understand it. The buffer hasn't changed, neither have the start/size. The fact that you were getting a dirty bit set before was an accident of implementation - had the test not been calling piglit_draw_rect(), glBufferData() would not have been called, and you would not have seen the new bindings, which would not have triggered the unnecessary dirtying of the UBO. It's really unfortunate that piglit_draw_rect() ends up dirtying so much state - this can end up papering over other issues. > > - 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.) I believe that your transfer_unmap/flush logic needs to become smarter. Especially in the situation of persistently and coherently-mapped buffers, you really need to keep track of this stuff. (And you appear to support ARB_buffer_storage). Cheers, -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev