[Mesa-dev] [PATCH] mesa: rewrite accum buffer support

[Eric Anholt]

On Mon,  5 Dec 2011 20:41:25 -0700, Brian Paul <brianp at vmware.com> wrote:
> Implemented in terms of renderbuffer mapping/unmapping and format
> packing/unpacking functions.
> 
> The swrast and state tracker code for implementing accumulation are
> unused and will be removed in the next commit.
> 
> v2: don't use memcpy() in _mesa_clear_accum_buffer()
> v3: don't allocate MAX_WIDTH arrays, be more careful with mapping flags

Minor comments, then:

Reviewed-by: Eric Anholt <eric at anholt.net>

> ---
>  src/mesa/SConscript                      |    2 -
>  src/mesa/drivers/common/driverfuncs.c    |    3 +-
>  src/mesa/drivers/dri/intel/intel_pixel.c |    2 +-
>  src/mesa/main/accum.c                    |  388 +++++++++++++++++++++++++++++-
>  src/mesa/main/accum.h                    |   18 ++
>  src/mesa/sources.mak                     |    2 -
>  src/mesa/state_tracker/st_cb_clear.c     |    5 +-
>  src/mesa/state_tracker/st_cb_fbo.c       |   21 ++
>  src/mesa/state_tracker/st_context.c      |    5 +-
>  src/mesa/swrast/s_clear.c                |    5 +-
>  10 files changed, 436 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/SConscript b/src/mesa/SConscript
> index 5c50189..633cb9c 100644
> --- a/src/mesa/SConscript
> +++ b/src/mesa/SConscript
> @@ -153,7 +153,6 @@ math_sources = [
>  swrast_sources = [
>      'swrast/s_aaline.c',
>      'swrast/s_aatriangle.c',
> -    'swrast/s_accum.c',
>      'swrast/s_alpha.c',
>      'swrast/s_atifragshader.c',
>      'swrast/s_bitmap.c',
> @@ -241,7 +240,6 @@ statetracker_sources = [
>      'state_tracker/st_atom_stipple.c',
>      'state_tracker/st_atom_texture.c',
>      'state_tracker/st_atom_viewport.c',
> -    'state_tracker/st_cb_accum.c',
>      'state_tracker/st_cb_bitmap.c',
>      'state_tracker/st_cb_blit.c',
>      'state_tracker/st_cb_bufferobjects.c',
> diff --git a/src/mesa/drivers/common/driverfuncs.c b/src/mesa/drivers/common/driverfuncs.c
> index 50fcede..0785cff 100644
> --- a/src/mesa/drivers/common/driverfuncs.c
> +++ b/src/mesa/drivers/common/driverfuncs.c
> @@ -25,6 +25,7 @@
>  
>  #include "main/glheader.h"
>  #include "main/imports.h"
> +#include "main/accum.h"
>  #include "main/arrayobj.h"
>  #include "main/context.h"
>  #include "main/framebuffer.h"
> @@ -80,7 +81,7 @@ _mesa_init_driver_functions(struct dd_function_table *driver)
>  
>     /* framebuffer/image functions */
>     driver->Clear = _swrast_Clear;
> -   driver->Accum = _swrast_Accum;
> +   driver->Accum = _mesa_accum;
>     driver->RasterPos = _tnl_RasterPos;
>     driver->DrawPixels = _swrast_DrawPixels;
>     driver->ReadPixels = _mesa_readpixels;
> diff --git a/src/mesa/drivers/dri/intel/intel_pixel.c b/src/mesa/drivers/dri/intel/intel_pixel.c
> index ae2ac05..4f665a7 100644
> --- a/src/mesa/drivers/dri/intel/intel_pixel.c
> +++ b/src/mesa/drivers/dri/intel/intel_pixel.c
> @@ -157,7 +157,7 @@ intel_check_blit_format(struct intel_region * region,
>  void
>  intelInitPixelFuncs(struct dd_function_table *functions)
>  {
> -   functions->Accum = _swrast_Accum;
> +   functions->Accum = _mesa_accum;
>     if (!getenv("INTEL_NO_BLIT")) {
>        functions->Bitmap = intelBitmap;
>        functions->CopyPixels = intelCopyPixels;
> diff --git a/src/mesa/main/accum.c b/src/mesa/main/accum.c
> index d7ed3a8..5fe8dc6 100644
> --- a/src/mesa/main/accum.c
> +++ b/src/mesa/main/accum.c
> @@ -24,7 +24,10 @@
>  
>  #include "glheader.h"
>  #include "accum.h"
> +#include "condrender.h"
>  #include "context.h"
> +#include "format_unpack.h"
> +#include "format_pack.h"
>  #include "imports.h"
>  #include "macros.h"
>  #include "mfeatures.h"
> @@ -101,7 +104,7 @@ _mesa_Accum( GLenum op, GLfloat value )
>        return;
>  
>     if (ctx->RenderMode == GL_RENDER) {
> -      ctx->Driver.Accum(ctx, op, value);
> +      _mesa_accum(ctx, op, value);
>     }

If we're going to hard-wire this, we should just ditch the
ctx->Driver.Accum hook, right?

(I'm neutral on it -- it seems like something where metaops could be
built, I just don't see anyone I know caring enough to ever do it).

> +         if (rgba)
> +            free(rgba);
> +         if (dest)
> +            free(dest);

free() happily takes NULL pointers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111206/eead4a7f/attachment.pgp>