On 22 May 2012 11:08, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On 05/14/2012 03:47 PM, Paul Berry wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> This patch implements clipping and scissoring of the destination rect for blits that use the blorp engine (e.g. MSAA blits). --- src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 72 ++++++++++++++++++++++++-- 1 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp index ee65cd6..223d8c1 100644 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp @@ -50,6 +50,63 @@ fixup_mirroring(bool&mirror, GLint&coord0, GLint&coord1) } +/** + * Adjust {src,dst}_x{0,1} to account for clipping and scissoring of + * destination coordinates. + * + * Return true if there is still blitting to do, false if all pixels got + * rejected by the clip and/or scissor. + * + * For clarity, the nomenclature of this function assumes we are clipping and + * scissoring the X coordinate; the exact same logic applies for Y + * coordinates. + */ +static inline bool +clip_or_scissor(bool mirror, GLint&src_x0, GLint&src_x1, GLint&dst_x0, + GLint&dst_x1, GLint fb_xmin, GLint fb_xmax) </blockquote> </div></div> Are fb_xmin and fb_xmax the dimensions of the framebuffer, or are they the scissor rectangle?</blockquote><div> They're the _Xmin and _Xmax values from struct gl_framebuffer, which are the intersection of the framebuffer bounds and the scissor rectangle (see mtypes.h line 2626). I'll add a comment to clarify this. </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +{ + /* If we are going to scissor everything away, stop. */ + if (!(fb_xmin< fb_xmax&& + dst_x0< fb_xmax&& + fb_xmin< dst_x1&& + dst_x0< dst_x1)) { + return false; + } </blockquote> </div> Isn't this handled by core Mesa before execution gets into the driver?</blockquote><div> Not that I can find. The only checks that happen in core mesa are in _mesa_BlitFramebufferEXT (fbobject.c), and they're all error checks (e.g. make sure the framebuffers are complete, make sure color types are compatible, etc.) </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + /* Clip the destination rectangle, and keep track of how many pixels we + * clipped off of the left and right sides of it. + */ + GLint pixels_clipped_left = 0; + GLint pixels_clipped_right = 0; + if (dst_x0< fb_xmin) { + pixels_clipped_left = fb_xmin - dst_x0; + dst_x0 = fb_xmin; + } + if (fb_xmax< dst_x1) { + pixels_clipped_right = dst_x1 - fb_xmax; + dst_x1 = fb_xmax; + } + + /* If we are mirrored, then before applying pixels_clipped_{left,right} to + * the source coordinates, we need to flip them to account for the + * mirroring. + */ + if (mirror) { + GLint tmp = pixels_clipped_left; + pixels_clipped_left = pixels_clipped_right; + pixels_clipped_right = tmp; + } + + /* Adjust the source rectangle to remove the pixels corresponding to those + * that were clipped/scissored out of the destination rectangle. + */ + src_x0 += pixels_clipped_left; + src_x1 -= pixels_clipped_right; </blockquote> </div></div> Is this going to cause coordinate rounding errors with linear sampling?</blockquote><div> It would for scaling blits, but currently brw_blorp_blit.cpp only supports 1:1 blits (because that's all that is required for MSAA). </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + return true; +} + + static bool try_blorp_blit(struct intel_context *intel, GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, @@ -154,14 +211,19 @@ try_blorp_blit(struct intel_context *intel, if (width != dstX1 - dstX0) return false; if (height != dstY1 - dstY0) return false; - /* Make sure width and height don't need to be clipped or scissored. - * TODO: support clipping and scissoring. + /* If the destination rectangle needs to be clipped or scissored, do so. */ + if (!(clip_or_scissor(mirror_x, srcX0, srcX1, dstX0, dstX1, + draw_fb->_Xmin, draw_fb->_Xmax)&& + clip_or_scissor(mirror_y, srcY0, srcY1, dstY0, dstY1, + draw_fb->_Ymin, draw_fb->_Ymax))) { + /* Everything got clipped/scissored away, so the blit was successful. */ + return true; + } + + /* TODO: Clipping the source rectangle is not yet implemented. */ if (srcX0< 0 || (GLuint) srcX1> read_fb->Width) return false; if (srcY0< 0 || (GLuint) srcY1> read_fb->Height) return false; - if (dstX0< 0 || (GLuint) dstX1> draw_fb->Width) return false; - if (dstY0< 0 || (GLuint) dstY1> draw_fb->Height) return false; - if (ctx->Scissor.Enabled) return false; /* Get ready to blit. This includes depth resolving the src and dst * buffers if necessary. </blockquote> </div></div></blockquote></div>