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. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev