Am 10.06.2016 um 05:14 schrieb Ilia Mirkin: > On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger <srol...@vmware.com> > wrote: >> Am 10.06.2016 um 04:58 schrieb Roland Scheidegger: >>> 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. >>> >>> - 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.) >>> >> >> >> Err actually this analysis is flawed. >> llvmpipe would indeed check in llvmpipe_transfer_map() if a currently >> bound constant buffer is changed and hence set LP_NEW_CONSTANTS accordingly. >> But it doesn't because the buffer doesn't have the >> PIPE_BIND_CONSTANT_BUFFER usage flag set (so, strictly speaking, it >> would be illegal to bind it as such, but this is never enforced). In >> fact it doesn't have _any_ bind flag set. >> So I blame the GL api, but I don't know how to fix this mess cleanly (we >> don't really want to ignore bind flags completely). > > Ah yeah, rookie mistake. pipe_resource->bind is only there to confuse > you. If you use it for anything after resource creation, that's a bug. >
This was designed in gallium with dx10 in mind, which has a proper api for this. But I suppose the only way to fix it in the mesa state tracker would be to just set ALL possible bind flags for buffers always... Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
