On Sun, 22 Sep 2019 15:24:10 +0200 Boris Brezillon <boris.brezil...@collabora.com> wrote:
> On Sun, 22 Sep 2019 08:38:30 -0400 > Alyssa Rosenzweig <aly...@rosenzweig.io> wrote: > > > > > To be clear, if we have a batch and do the following operations: > > > > > > > > clear red > > > > draw 1 > > > > clear green > > > > draw 2 > > > > flush > > > > > > > > All we should see is #2 on a green background, which this patch handles > > > > by the second clear invalidating all the clears/draws that came before > > > > it (provided there is no flush in between). > > > > > > > > I might just be tripped up by the "freeze" name. That really means throw > > > > away / free here, I guess? > > > > > > Nope. Freeze means "stop queuing new draws to this batch". I guess we > > > could free the batch as well if the result of the previous draws/clear > > > are really overwritten by this new clear, but that implies checking the > > > new clear flags to make sure they target the same depth/stencil/color > > > combination. On the other hand, I'm wondering if it's really the > > > driver's job to try to optimize silly things the apps might do. I mean, > > > the sequence you describe does not look like a wise thing to do since > > > the "clear red+draw 1" end up being overwritten by "clear green + draw > > > 2". > > > > I'm quite confused how this patch works, then. > > > > A few thoughts: if the app clears all buffers in the middle, then yes > > it's silly and yes we may as well optimize it out. (Should that be a thing > > GL > > drivers have to do? I mean, if the other drivers are too...) > > > > If the sequence is more like: > > > > clear all buffers > > draw 1 > > clear color buffer (preserve depth stencil) > > draw 2 > > flush > > > > That second clear should really be done by drawing a full screen quad, > > just like if we were wallpapering, except loading its colour from a > > uniform instead of a texture. > > > > Similarly, a depth-only clear mid-frame can be emulated by drawing a > > full-screen quad with the gl_Position.zw components juryrigged to the > > desired depth components, and disabling colour draws by setting the > > colour mask to 0x0. That also means you can skip having any shader at > > all (literally set the shader pointer to 0x0) so that's faster. > > > > Finally, a stencil-only clear can occur similarly playing tricks with > > the stencil test parameters. > > > > I suspect u_blitter or mesa/st is capable of doing these sorts of tricks > > on our behalf, but I have not researched it extensively. > > AFAIU, mesa/st does not transform the clear into a quad-draw for > depth/stencil only clears, it only does that for the color(s)-masked > case. That's certainly something we can do in Panfrost if we want to > avoid creating a new batch in such situations though. One more thing: optimization of the above scenario is probably something we'll want to have at some point, but I think the current patch is worth applying in the meantime. All this patch does is enforcing ordering of clears/draws to make sure the end result matches users expectation. > > > > > In any case, for a partial clear mid-frame, we would much rather do: > > > > clear all buffers > > draw 1 > > draw fullscreen quad (disable Z/S writes) > > draw 2 > > flush > > > > than what it sounds like this patch does > > > > clear all buffers > > draw 1 > > flush > > > > clear all buffers > > wallpaper > > draw 2 > > flush > > > > Please do correct me if I've misunderstood the logic here. > > > There's no flush introduced by this patch, it just adds a dep between > batch 2 and 1: > > batch1: clear all buffers > draw 1 > > batch2: clear all buffers > wallpaper > draw 2 > > flush (actually flushes batch 1 and 2) > > with batch2 --depends-on--> batch1 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
