On Tue, 2018-11-13 at 19:07 -0500, Ilia Mirkin wrote: > On Tue, Nov 13, 2018 at 6:50 PM Rob Clark <robdcl...@gmail.com> > wrote: > > On Tue, Nov 13, 2018 at 6:19 PM Eric Anholt <e...@anholt.net> > > wrote: > > > Rob Clark <robdcl...@gmail.com> writes: > > > > > > > On Tue, Nov 13, 2018 at 5:25 PM Eric Anholt <e...@anholt.net> > > > > wrote: > > > > > Rob Clark <robdcl...@gmail.com> writes: > > > > > > > > > > > If we can't clear all the buffers with pctx->clear() (say, > > > > > > for example, > > > > > > because of ColorMask), push the buffers we *can* clear with > > > > > > pctx->clear() > > > > > > first. Tilers want to see clears coming before draws to > > > > > > enable fast- > > > > > > paths, and clearing one of the attachments with a quad-draw > > > > > > first > > > > > > confuses that logic. > > > > > > > > > > Oh, nice! > > > > > > > > > > Reviewed-by: Eric Anholt <e...@anholt.net> > > > > > > > > > > Though it feels pretty silly that the ->clear() caller needs > > > > > a > > > > > clear_with_quad implementation when the ->clear() > > > > > implementation in the > > > > > driver also needs a clear_with_quad implementation for non- > > > > > fast-cleared > > > > > buffers. :/ > > > > > > > > hmm, so perhaps one easy option is to change pctx->clear() to > > > > return a > > > > boolean, so driver can return false to ask the state tracker to > > > > do a > > > > clear_with_quad().. maybe that would be a first step towards > > > > allowing > > > > driver to handle clears w/ colormask and possibly scissor > > > > (although > > > > for the later, plus > > > > glInvalidateFramebuffer()/glInvalidateSubFramebuffer(), I was > > > > thinking > > > > of pctx->invalidate_surface()/pctx->invalidate_sub_surface()). > > > > > > I was thinking you'd return the mask of what buffers you couldn't > > > (fast) > > > clear. > > > > yeah, makes sense.. I kinda came to same conclusion when I started > > thinking some drivers might not want us to split up the clear per > > attachment.. still not quite sure about adding scissor/colormask, > > might end up needing a pipe cap so st_Clear() would know to flush > > the > > corresponding state down to driver. I guess low hanging fruit is > > to > > not change the definition of pctx->clear() but just let driver ask > > for > > fallback path for some/all attachments. > > You could also create a pipe_clear_info which would take that data > directly and let the driver worry about it. FWIW, nvidia command > stream clears can take into account stencil, scissors, window > rectangles, color masks - maybe everything that st_Clear needs to > worry about. It never seemed important enough to address myself, but > I'll happily go along for the ride.
Just another suggestion in the bag: Vulkan supports three kinds of clears: - Render pass clears; used at the start of a render pass, no scissoring or color-masking. - vkCmdClearAttachments: used inside render-passes, allows to specify an arbitrary number of scissor-rectangles, no color-maksing. - vkCmdClearColorImage / vkCmdClearDepthStencilImage: used outside render-passes, no scissoring or color-masking. All of the Vulkan clears are "per-attachment" in a sense; - Render pass clears / vkCmdClearAttachments can select any of the attachments in the current framebuffer - vkCmdClearColorImage / vkCmdClearDepthStencilImage can clear individual images, regardless of framebuffers. The reason I'm bringing this up, is that I assume the Vulkan WG has had long discussions about how to efficiently support common needs for clears on all relevant hardware. It seems a bit logical (to me, at least) that a color-masked clear is always a quad. Color masking makes it not really a clear-operation, but more like a strange blend, and color-masking is a graphics-pipeline operation. I doubt we'd ever care about stencil, as even GL doesn't. So: - pipe_context::clear_render_target corresponds to vkCmdClearColorImage (with the exception that clear_render_target can specify a rectangle). - pipe_context::clear_depth_stencil corresponds to vkCmdClearDepthStencilImage (with the exception that clear_depth_stencil can specify a rectangle). - pipe_context::clear corresponds to render pass clears I think the only case we currently miss compared to what Vulkan provides is a non-quad version of scissored clear (vkCmdClearAttachments). So perhaps we should add support for that somehow? Apart from that, it sounds like the suggestion of returning a mask and falling back ti quads for harware that can't support this is a good suggestion to me. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
