[Mesa-dev] [PATCH] i965/blorp: Implement destination clipping and scissoring

[Paul Berry]

On 22 May 2012 11:08, Ian Romanick <idr at freedesktop.org> wrote:

> On 05/14/2012 03:47 PM, Paul Berry wrote:
>
>> 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)
>>
>
> Are fb_xmin and fb_xmax the dimensions of the framebuffer, or are they the
> scissor rectangle?


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.


>
>
>  +{
>> +   /* 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;
>> +   }
>>
>
> Isn't this handled by core Mesa before execution gets into the driver?


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.)


>
>
>  +
>> +   /* 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;
>>
>
> Is this going to cause coordinate rounding errors with linear sampling?


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).


>
>
>  +
>> +   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.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120523/bc02af88/attachment.htm>