Re: [Mesa-dev] [PATCH 2/3] swrast: rewrite color buffer clearing to use Map/UnmapRenderbuffer()

2011-12-12 Thread Eric Anholt
On Sat, 10 Dec 2011 12:00:53 -0700, Brian Paul bri...@vmware.com wrote:
 -   /* Note that masking will change the color values, but only the
 -* channels for which the write mask is GL_FALSE.  The channels
 -* which which are write-enabled won't get modified.
 -*/
 -   for (i = 0; i  height; i++) {
 -  span.x = x;
 -  span.y = y + i;
 -  _swrast_mask_rgba_span(ctx, rb, span, buf);
 -  /* write masked row */
 -  rb-PutRow(ctx, rb, width, x, y + i, span.array-rgba, NULL);
 +   if (doMasking) {
 +  /* Convert the boolean mask to a color */

I think this would be more informative as

/* Convert the boolean mask to a color value that will be packed to
 * produce the bitmask for the renderbuffer's format.
 */

but overall, this looks like a pretty nice (if tricky) way to handle all
these formats.  Reviewed-by: Eric Anholt e...@anholt.net


pgpDwhlmTYsUB.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/3] swrast: rewrite color buffer clearing to use Map/UnmapRenderbuffer()

2011-12-12 Thread Brian Paul
On Mon, Dec 12, 2011 at 10:38 AM, Eric Anholt e...@anholt.net wrote:
 On Sat, 10 Dec 2011 12:00:53 -0700, Brian Paul bri...@vmware.com wrote:
 -   /* Note that masking will change the color values, but only the
 -    * channels for which the write mask is GL_FALSE.  The channels
 -    * which which are write-enabled won't get modified.
 -    */
 -   for (i = 0; i  height; i++) {
 -      span.x = x;
 -      span.y = y + i;
 -      _swrast_mask_rgba_span(ctx, rb, span, buf);
 -      /* write masked row */
 -      rb-PutRow(ctx, rb, width, x, y + i, span.array-rgba, NULL);
 +   if (doMasking) {
 +      /* Convert the boolean mask to a color */

 I think this would be more informative as

 /* Convert the boolean mask to a color value that will be packed to
  * produce the bitmask for the renderbuffer's format.
  */

 but overall, this looks like a pretty nice (if tricky) way to handle all
 these formats.  Reviewed-by: Eric Anholt e...@anholt.net

Actually, I found a couple issues with the code when I was looking at
it yesterday.  I'll post an updated patch later.

I'll update the comments too.

-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/3] swrast: rewrite color buffer clearing to use Map/UnmapRenderbuffer()

2011-12-10 Thread Brian Paul
---
 src/mesa/swrast/s_clear.c |  234 +++--
 1 files changed, 119 insertions(+), 115 deletions(-)

diff --git a/src/mesa/swrast/s_clear.c b/src/mesa/swrast/s_clear.c
index 778911f..30c89bc 100644
--- a/src/mesa/swrast/s_clear.c
+++ b/src/mesa/swrast/s_clear.c
@@ -24,131 +24,149 @@
 
 #include main/glheader.h
 #include main/accum.h
-#include main/colormac.h
 #include main/condrender.h
+#include main/format_pack.h
 #include main/macros.h
 #include main/imports.h
 #include main/mtypes.h
 
 #include s_context.h
 #include s_depth.h
-#include s_masking.h
 #include s_stencil.h
 
 
 /**
- * Clear the color buffer when glColorMask is in effect.
+ * Clear an rgba color buffer with masking if needed.
  */
 static void
-clear_rgba_buffer_with_masking(struct gl_context *ctx, struct gl_renderbuffer 
*rb,
-   GLuint buf)
+clear_rgba_buffer(struct gl_context *ctx, struct gl_renderbuffer *rb,
+  const GLubyte colorMask[4])
 {
const GLint x = ctx-DrawBuffer-_Xmin;
const GLint y = ctx-DrawBuffer-_Ymin;
const GLint height = ctx-DrawBuffer-_Ymax - ctx-DrawBuffer-_Ymin;
const GLint width  = ctx-DrawBuffer-_Xmax - ctx-DrawBuffer-_Xmin;
-   SWspan span;
-   GLint i;
-
-   ASSERT(rb-PutRow);
-
-   /* Initialize color span with clear color */
-   /* XXX optimize for clearcolor == black/zero (bzero) */
-   INIT_SPAN(span, GL_BITMAP);
-   span.end = width;
-   span.arrayMask = SPAN_RGBA;
-   span.array-ChanType = rb-DataType;
-   if (span.array-ChanType == GL_UNSIGNED_BYTE) {
-  GLubyte clearColor[4];
-  _mesa_unclamped_float_rgba_to_ubyte(clearColor, ctx-Color.ClearColor.f);
-  for (i = 0; i  width; i++) {
- COPY_4UBV(span.array-rgba[i], clearColor);
-  }
-   }
-   else if (span.array-ChanType == GL_UNSIGNED_SHORT) {
-  GLushort clearColor[4];
-  UNCLAMPED_FLOAT_TO_USHORT(clearColor[RCOMP], ctx-Color.ClearColor.f[0]);
-  UNCLAMPED_FLOAT_TO_USHORT(clearColor[GCOMP], ctx-Color.ClearColor.f[1]);
-  UNCLAMPED_FLOAT_TO_USHORT(clearColor[BCOMP], ctx-Color.ClearColor.f[2]);
-  UNCLAMPED_FLOAT_TO_USHORT(clearColor[ACOMP], ctx-Color.ClearColor.f[3]);
-  for (i = 0; i  width; i++) {
- COPY_4V_CAST(span.array-rgba[i], clearColor, GLchan);
-  }
-   }
-   else {
-  ASSERT(span.array-ChanType == GL_FLOAT);
-  for (i = 0; i  width; i++) {
- UNCLAMPED_FLOAT_TO_CHAN(span.array-rgba[i][0], 
ctx-Color.ClearColor.f[0]);
- UNCLAMPED_FLOAT_TO_CHAN(span.array-rgba[i][1], 
ctx-Color.ClearColor.f[1]);
- UNCLAMPED_FLOAT_TO_CHAN(span.array-rgba[i][2], 
ctx-Color.ClearColor.f[2]);
- UNCLAMPED_FLOAT_TO_CHAN(span.array-rgba[i][3], 
ctx-Color.ClearColor.f[3]);
-  }
+   const GLuint pixelSize = _mesa_get_format_bytes(rb-Format);
+   const GLboolean doMasking = (colorMask[0] == 0 ||
+colorMask[1] == 0 ||
+colorMask[2] == 0 ||
+colorMask[3] == 0);
+   const GLfloat (*clearColor)[4] =
+  (const GLfloat (*)[4]) ctx-Color.ClearColor.f;
+   GLfloat maskColor[4];
+   GLubyte *map;
+   GLint rowStride;
+   GLint i, j;
+
+   /* map dest buffer */
+   ctx-Driver.MapRenderbuffer(ctx, rb, x, y, width, height,
+   GL_MAP_WRITE_BIT,
+   map, rowStride);
+   if (!map) {
+  _mesa_error(ctx, GL_OUT_OF_MEMORY, glClear(color));
+  return;
}
 
-   /* Note that masking will change the color values, but only the
-* channels for which the write mask is GL_FALSE.  The channels
-* which which are write-enabled won't get modified.
-*/
-   for (i = 0; i  height; i++) {
-  span.x = x;
-  span.y = y + i;
-  _swrast_mask_rgba_span(ctx, rb, span, buf);
-  /* write masked row */
-  rb-PutRow(ctx, rb, width, x, y + i, span.array-rgba, NULL);
+   if (doMasking) {
+  /* Convert the boolean mask to a color */
+  maskColor[0] = colorMask[0] ? 1.0 : 0.0;
+  maskColor[1] = colorMask[1] ? 1.0 : 0.0;
+  maskColor[2] = colorMask[2] ? 1.0 : 0.0;
+  maskColor[3] = colorMask[3] ? 1.0 : 0.0;
}
-}
 
+   /* for 1, 2, 4-byte clearing */
+#define SIMPLE_TYPE_CLEAR(TYPE) \
+   do { \
+  TYPE pixel, pixelMask;\
+  _mesa_pack_float_rgba_row(rb-Format, 1, clearColor, pixel); \
+  if (doMasking) {  \
+ _mesa_pack_float_rgba_row(rb-Format, 1,   \
+(const GLfloat (*)[4]) maskColor, pixelMask);  \
+ pixel = ~pixelMask;   \
+  } \
+  for (i = 0; i  height; i++) {\
+ TYPE *row =