Re: [Mesa-dev] [PATCH v2 5/5] meta: Add a meta implementation of GL_ARB_clear_texture

2014-07-04 Thread Neil Roberts 
Pohjolainen, Topi topi.pohjolai...@intel.com writes:

 Oh, I didn't realize that. Should we fix and put this into
 _mesa_meta_begin/_mesa_meta_end instead?

Yes, perhaps that would be a good idea. I will try writing a separate
patch for that.

Thinking about it a bit more I also realised I'm not handling sRGB
textures correctly. I need to ensure that GL_FRAMEBUFFER_SRGB is
disabled otherwise it will apply the sRGB conversion when doing the
clear. I also now realise that _mesa_unpack_uint_rgba_row unapplies the
sRGB conversion. I am using that to calculate the value to give to
glClearColor so I need to stop it from doing that too.

I was wondering whether we should also ensure that GL_DITHER is disabled
because it affects glClear. For example if we were rendering to a 16-bit
texture with a clear color that is in-between values we would want it to
pick a single value for all the texels instead of dithering. However in
practice I don't think this matters because the clear value is first
converted to the same format as the texture before being passed to the
driver so it should be impossible to end up with a clear value that
would cause dithering. It might be nice to disable it anyway though just
to be on the safe side.

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

Re: [Mesa-dev] [PATCH v2 5/5] meta: Add a meta implementation of GL_ARB_clear_texture

2014-07-03 Thread Pohjolainen, Topi 
On Wed, Jul 02, 2014 at 07:00:06PM +0100, Neil Roberts wrote:
 Adds an implementation of the ClearTexSubImage driver entry point that tries
 to set up an FBO to render to the texture and then calls glClear with a
 scissor to perform the actual clear. If an FBO can't be created for the
 texture then it will fall back to using _mesa_store_ClearTexSubImage.
 
 When used in combination with _mesa_store_ClearTexSubImage this
 should provide an implementation that works for all DRI-based drivers. However
 as this has only been tested with the i965 driver it is currently only enabled
 there.
 
 v2: Only enable the extension for the i965 driver instead of all DRI drivers.
 Remove an unnecessary goto. Don't require GL_ARB_framebuffer_object. Add
 some more comments.
 ---
  src/mesa/drivers/common/driverfuncs.c|   1 +
  src/mesa/drivers/common/meta.c   | 163 
 +++
  src/mesa/drivers/common/meta.h   |   7 ++
  src/mesa/drivers/dri/i965/intel_extensions.c |   1 +
  4 files changed, 172 insertions(+)
 
 diff --git a/src/mesa/drivers/common/driverfuncs.c 
 b/src/mesa/drivers/common/driverfuncs.c
 index 6ece5d8..4f0f7a6 100644
 --- a/src/mesa/drivers/common/driverfuncs.c
 +++ b/src/mesa/drivers/common/driverfuncs.c
 @@ -95,6 +95,7 @@ _mesa_init_driver_functions(struct dd_function_table 
 *driver)
 driver-TexImage = _mesa_store_teximage;
 driver-TexSubImage = _mesa_store_texsubimage;
 driver-GetTexImage = _mesa_meta_GetTexImage;
 +   driver-ClearTexSubImage = _mesa_meta_ClearTexSubImage;
 driver-CopyTexSubImage = _mesa_meta_CopyTexSubImage;
 driver-GenerateMipmap = _mesa_meta_GenerateMipmap;
 driver-TestProxyTexImage = _mesa_test_proxy_teximage;
 diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
 index f1f5729..141178e 100644
 --- a/src/mesa/drivers/common/meta.c
 +++ b/src/mesa/drivers/common/meta.c
 @@ -40,6 +40,7 @@
  #include main/blit.h
  #include main/bufferobj.h
  #include main/buffers.h
 +#include main/clear.h
  #include main/colortab.h
  #include main/condrender.h
  #include main/depth.h
 @@ -47,6 +48,7 @@
  #include main/fbobject.h
  #include main/feedback.h
  #include main/formats.h
 +#include main/format_unpack.h
  #include main/glformats.h
  #include main/image.h
  #include main/macros.h
 @@ -71,6 +73,7 @@
  #include main/teximage.h
  #include main/texparam.h
  #include main/texstate.h
 +#include main/texstore.h
  #include main/transformfeedback.h
  #include main/uniforms.h
  #include main/varray.h
 @@ -3339,3 +3342,163 @@ _mesa_meta_DrawTex(struct gl_context *ctx, GLfloat x, 
 GLfloat y, GLfloat z,
  
 _mesa_meta_end(ctx);
  }
 +
 +static void
 +set_color_clear_value(struct gl_context *ctx,
 +  mesa_format format,
 +  const GLvoid *clearValue)
 +{
 +   if (clearValue == 0) {
 + memset(ctx-Color.ClearColor, 0, sizeof ctx-Color.ClearColor);
 +   } else {
 +  switch (_mesa_get_format_datatype(format)) {
 +  case GL_UNSIGNED_INT:
 +  case GL_INT:
 + _mesa_unpack_uint_rgba_row(format, 1, clearValue,
 +(GLuint (*)[4]) 
 ctx-Color.ClearColor.ui);
 + break;
 +  default:
 + _mesa_unpack_rgba_row(format, 1, clearValue,
 +   (GLfloat (*)[4]) ctx-Color.ClearColor.f);
 + break;
 +  }
 +   }
 +}
 +
 +static bool
 +cleartexsubimage_using_fbo_for_zoffset(struct gl_context *ctx,
 +   struct gl_texture_image *texImage,
 +   GLint xoffset, GLint yoffset,
 +   GLint zoffset,
 +   GLsizei width, GLsizei height,
 +   const GLvoid *clearValue)
 +{
 +   GLuint fbo;
 +   bool success = false;
 +   GLbitfield mask;
 +   GLenum status;
 +
 +   _mesa_GenFramebuffers(1, fbo);
 +   _mesa_BindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
 +
 +   if (texImage-_BaseFormat == GL_DEPTH_STENCIL ||
 +   texImage-_BaseFormat == GL_DEPTH_COMPONENT) {
 +  _mesa_meta_bind_fbo_image(GL_DEPTH_ATTACHMENT, texImage, zoffset);
 +  mask = GL_DEPTH_BUFFER_BIT;
 +
 +  if (clearValue) {
 + GLuint depthStencilValue[2];
 + GLfloat depthValue;
 +
 + /* Convert the clearValue from whatever format it's in to a floating
 +  * point value for the depth and an integer value for the stencil
 +  * index so we can set the clear values */

I wonder what I was reading the first time, this call is clearly the right
thing to do as it handles all the combinations - sorry for the confusion.

Small nit for the comment style here and in the rest of the patch, in mesa
it's:

/* Convert the clearValue from whatever format it's in to a floating
 * point value for the depth and an integer value for the stencil
 * index so we can set the

[Mesa-dev] [PATCH v2 5/5] meta: Add a meta implementation of GL_ARB_clear_texture

2014-07-02 Thread Neil Roberts 
Adds an implementation of the ClearTexSubImage driver entry point that tries
to set up an FBO to render to the texture and then calls glClear with a
scissor to perform the actual clear. If an FBO can't be created for the
texture then it will fall back to using _mesa_store_ClearTexSubImage.

When used in combination with _mesa_store_ClearTexSubImage this
should provide an implementation that works for all DRI-based drivers. However
as this has only been tested with the i965 driver it is currently only enabled
there.

v2: Only enable the extension for the i965 driver instead of all DRI drivers.
Remove an unnecessary goto. Don't require GL_ARB_framebuffer_object. Add
some more comments.
---
 src/mesa/drivers/common/driverfuncs.c|   1 +
 src/mesa/drivers/common/meta.c   | 163 +++
 src/mesa/drivers/common/meta.h   |   7 ++
 src/mesa/drivers/dri/i965/intel_extensions.c |   1 +
 4 files changed, 172 insertions(+)

diff --git a/src/mesa/drivers/common/driverfuncs.c 
b/src/mesa/drivers/common/driverfuncs.c
index 6ece5d8..4f0f7a6 100644
--- a/src/mesa/drivers/common/driverfuncs.c
+++ b/src/mesa/drivers/common/driverfuncs.c
@@ -95,6 +95,7 @@ _mesa_init_driver_functions(struct dd_function_table *driver)
driver-TexImage = _mesa_store_teximage;
driver-TexSubImage = _mesa_store_texsubimage;
driver-GetTexImage = _mesa_meta_GetTexImage;
+   driver-ClearTexSubImage = _mesa_meta_ClearTexSubImage;
driver-CopyTexSubImage = _mesa_meta_CopyTexSubImage;
driver-GenerateMipmap = _mesa_meta_GenerateMipmap;
driver-TestProxyTexImage = _mesa_test_proxy_teximage;
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index f1f5729..141178e 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -40,6 +40,7 @@
 #include main/blit.h
 #include main/bufferobj.h
 #include main/buffers.h
+#include main/clear.h
 #include main/colortab.h
 #include main/condrender.h
 #include main/depth.h
@@ -47,6 +48,7 @@
 #include main/fbobject.h
 #include main/feedback.h
 #include main/formats.h
+#include main/format_unpack.h
 #include main/glformats.h
 #include main/image.h
 #include main/macros.h
@@ -71,6 +73,7 @@
 #include main/teximage.h
 #include main/texparam.h
 #include main/texstate.h
+#include main/texstore.h
 #include main/transformfeedback.h
 #include main/uniforms.h
 #include main/varray.h
@@ -3339,3 +3342,163 @@ _mesa_meta_DrawTex(struct gl_context *ctx, GLfloat x, 
GLfloat y, GLfloat z,
 
_mesa_meta_end(ctx);
 }
+
+static void
+set_color_clear_value(struct gl_context *ctx,
+  mesa_format format,
+  const GLvoid *clearValue)
+{
+   if (clearValue == 0) {
+ memset(ctx-Color.ClearColor, 0, sizeof ctx-Color.ClearColor);
+   } else {
+  switch (_mesa_get_format_datatype(format)) {
+  case GL_UNSIGNED_INT:
+  case GL_INT:
+ _mesa_unpack_uint_rgba_row(format, 1, clearValue,
+(GLuint (*)[4]) ctx-Color.ClearColor.ui);
+ break;
+  default:
+ _mesa_unpack_rgba_row(format, 1, clearValue,
+   (GLfloat (*)[4]) ctx-Color.ClearColor.f);
+ break;
+  }
+   }
+}
+
+static bool
+cleartexsubimage_using_fbo_for_zoffset(struct gl_context *ctx,
+   struct gl_texture_image *texImage,
+   GLint xoffset, GLint yoffset,
+   GLint zoffset,
+   GLsizei width, GLsizei height,
+   const GLvoid *clearValue)
+{
+   GLuint fbo;
+   bool success = false;
+   GLbitfield mask;
+   GLenum status;
+
+   _mesa_GenFramebuffers(1, fbo);
+   _mesa_BindFramebuffer(GL_DRAW_FRAMEBUFFER, fbo);
+
+   if (texImage-_BaseFormat == GL_DEPTH_STENCIL ||
+   texImage-_BaseFormat == GL_DEPTH_COMPONENT) {
+  _mesa_meta_bind_fbo_image(GL_DEPTH_ATTACHMENT, texImage, zoffset);
+  mask = GL_DEPTH_BUFFER_BIT;
+
+  if (clearValue) {
+ GLuint depthStencilValue[2];
+ GLfloat depthValue;
+
+ /* Convert the clearValue from whatever format it's in to a floating
+  * point value for the depth and an integer value for the stencil
+  * index so we can set the clear values */
+ _mesa_unpack_float_32_uint_24_8_depth_stencil_row(texImage-TexFormat,
+   1, /* n */
+   clearValue,
+   depthStencilValue);
+ /* We need a memcpy here instead of a cast because we need to
+  * reinterpret the bytes as a float rather than converting it */
+ memcpy(depthValue, depthStencilValue, sizeof depthValue);
+ ctx-Depth.Clear = depthValue;
+ ctx-Stencil.Clear = depthStencilValue[1]  0xff;
+