On 06/13/2016 10:04 AM, Marek Olšák wrote:
On Mon, Jun 13, 2016 at 5:17 PM, Roland Scheidegger <[email protected]> wrote:
Am 13.06.2016 um 14:53 schrieb Marek Olšák:
On Mon, Jun 13, 2016 at 1:14 PM, Jose Fonseca <[email protected]> wrote:
On 10/06/16 15:40, Roland Scheidegger wrote:
Am 10.06.2016 um 12:38 schrieb Marek Olšák:
On Fri, Jun 10, 2016 at 6:19 AM, Roland Scheidegger <[email protected]>
wrote:
Am 10.06.2016 um 05:14 schrieb Ilia Mirkin:
On Thu, Jun 9, 2016 at 11:13 PM, Roland Scheidegger
<[email protected]> 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 <[email protected]>
wrote:
On Wed, Jun 8, 2016 at 5:48 PM, Fredrik Höglund <[email protected]>
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...
Or you can assume bind == 0 means all flags are set.
Yes in this case, but I don't think that really helps. Even when using
the old gl api for setting this, the target (and hence the bind flag)
may be something completely different than the actual usage.
I'd rather have GL renderer set all possible bind flags.
And if the excess of bind flags leads certain drivers to significant
inefficiencies, then those drivers can internally track exactly where those
buffers/resources have been bound (similar to Roland's recent proposed patch
for llvmpipe.)
(And if it turns out that all drivers end up ignoring the state tracker bind
flags and just track bindings internally, we could even elimiate bind
altogether. But my hunch is that bind flags are useful. So one step at a
time.)
Honestly have anything except this unenforced contract would be better.
Setting PIPE_BIND_CONSTANT_BUFFER for all buffers will break r300g. We
should be careful and set only the possible flags that can occur based
on exposed extensions.
I think the state tracker could indeed restrict the bind flags based on
extensions supported, but not anything more. So, with most drivers it
would always have to set everything always for buffers (which includes
things as textures buffers).
I am not sure how well that would work with all drivers (svga for
instance has separate paths for buffers having or not CONSTANT bind
flags (the rest matter little), not sure if or how it would be affected).
Also, for some of the texture view / rt use cases for textures, the
state tracker does try to set the possibly required bind flags. But last
time I checked I think some of the issues were then coming from util
code actually (blits etc.).
Not saying it wouldn't be really nice to have it enforceable, but I've
just given up all hope - this is a problem since just about forever...
We can also remove pipe_resource::bind and move the remaining useful
flags to pipe_resource::flags. Those are:
- PIPE_BIND_SCANOUT - determines whether a texture can be displayed on
a monitor, very important
- PIPE_BIND_LINEAR - forces the linear layout for multi-GPU texture
sharing, very important
- PIPE_BIND_CONSTANT_BUFFER - a driver that doesn't support UBOs can
malloc the storage
- maybe a few others, but not many
Some other BIND flags are useful for is_format_supported. The rest can
be removed.
Let's not do this immediately. It would take some effort to update our
VMware driver.
-Brian
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
