<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Nov 14, 2018, 10:36 AM Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Wed, Nov 14, 2018 at 10:13 AM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank" rel="noreferrer">maraeo@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Wed, Nov 14, 2018, 7:54 AM Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank" rel="noreferrer">robdclark@gmail.com</a> wrote:<br>
>><br>
>> On Tue, Nov 13, 2018 at 9:54 PM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank" rel="noreferrer">maraeo@gmail.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> ><br>
>> > On Tue, Nov 13, 2018, 6:00 PM Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank" rel="noreferrer">robdclark@gmail.com</a> wrote:<br>
>> >><br>
>> >> On Tue, Nov 13, 2018 at 5:25 PM Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank" rel="noreferrer">eric@anholt.net</a>> wrote:<br>
>> >> ><br>
>> >> > Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank" rel="noreferrer">robdclark@gmail.com</a>> writes:<br>
>> >> ><br>
>> >> > > If we can't clear all the buffers with pctx->clear() (say, for example,<br>
>> >> > > because of ColorMask), push the buffers we *can* clear with pctx->clear()<br>
>> >> > > first.  Tilers want to see clears coming before draws to enable fast-<br>
>> >> > > paths, and clearing one of the attachments with a quad-draw first<br>
>> >> > > confuses that logic.<br>
>> >> ><br>
>> >> > Oh, nice!<br>
>> >> ><br>
>> >> > Reviewed-by: Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank" rel="noreferrer">eric@anholt.net</a>><br>
>> >> ><br>
>> >> > Though it feels pretty silly that the ->clear() caller needs a<br>
>> >> > clear_with_quad implementation when the ->clear() implementation in the<br>
>> >> > driver also needs a clear_with_quad implementation for non-fast-cleared<br>
>> >> > buffers.  :/<br>
>> >><br>
>> >> hmm, so perhaps one easy option is to change pctx->clear() to return a<br>
>> >> boolean, so driver can return false to ask the state tracker to do a<br>
>> ><br>
>> ><br>
>> > Ideally all pipe context functions should always return void to allow gallium multithreading.<br>
>><br>
>> Hmm, that is a bit awkward..  and a pipe cap is a bit restrictive, ie.<br>
>> say you can fast-clear only certain color formats, or other weird<br>
>> combination of restrictions.<br>
>><br>
>> I suppose a pctx->can_do_clear(..) which isn't multithreaded would be<br>
>> a bit more flexible than pipe caps.<br>
><br>
><br>
> can_do_clear(..) should be in pipe_screen. I wouldn't like to have context functions that return a value.<br>
<br>
that means we'd have to pass it framebuffer state, scissor state, and<br>
colormask.. which is a bit annoying and less of a trivial change to<br>
allow drivers to delete some code..<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">A driver can also set a flag in pipe_resource to indicate it's clearable. Drivers still have to implement the quad clear for ignorant state trackers.</div><div dir="auto"><br></div><div dir="auto">I would say that at least all desktop GPUs would want to use u_threaded_context, so it doesn't make sense to add any queries into pipe_context.</div><div dir="auto"><br></div><div dir="auto">Marek</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
BR,<br>
-R<br>
<br>
> Marek<br>
><br>
>><br>
>> > A pipe cap for colormasked and scissored clears would be better.<br>
>> ><br>
>> ><br>
>> >> clear_with_quad()..  maybe that would be a first step towards allowing<br>
>> >> driver to handle clears w/ colormask and possibly scissor (although<br>
>> >> for the later, plus<br>
>> >> glInvalidateFramebuffer()/glInvalidateSubFramebuffer(), I was thinking<br>
>> >> of pctx->invalidate_surface()/pctx->invalidate_sub_surface())<br>
>> ><br>
>> ><br>
>> > Pipe context already contains an invalidate function.<br>
>><br>
>> yeah, but it is at the level of pipe_resource, which isn't really what<br>
>> you want if only a certain level/layer of a resource is invalidated.<br>
>><br>
>> I'm still unsure about how I want to track this in the driver, the<br>
>> ideal case would be to track it in fd_surface, except that surfaces<br>
>> tend to be created transiently.  Still for glInvalidateFramebuffer()<br>
>> and friends, having an pctx->invalidate_surface() seems like the<br>
>> cleaner API.<br>
>><br>
>> BR,<br>
>> -R<br>
>><br>
>> ><br>
>> > Marek<br>
>> ><br>
>> >><br>
>> >> But either way, I guess this patch is a simple stop-gap solution.<br>
>> >><br>
>> >> BR,<br>
>> >> -R<br>
>> >> _______________________________________________<br>
>> >> mesa-dev mailing list<br>
>> >> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank" rel="noreferrer">mesa-dev@lists.freedesktop.org</a><br>
>> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div></div>