[Mesa-dev] Mesa (gallium-msaa): gallium: interface changes for multisampling
José Fonseca
jfonseca at vmware.com
Mon Apr 26 11:27:35 PDT 2010
Hi Roland,
Overall looks good. It's nice to finally have a way to export MSAA
capabilities, and see the pipe_surfaces use being more constrained.
A few comments inline, but no strong feelings so feel free to do as you
wish.
Jose
On Mon, 2010-04-26 at 11:05 -0700, Roland Scheidegger wrote:
> Module: Mesa
> Branch: gallium-msaa
> Commit: aac2cccccfd701ae8d7ce0813c28c64498d4a076
> URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=aac2cccccfd701ae8d7ce0813c28c64498d4a076
>
> Author: Roland Scheidegger <sroland at vmware.com>
> Date: Mon Apr 26 19:50:57 2010 +0200
>
> gallium: interface changes for multisampling
>
> add function to set sample mask, and state for alpha-to-coverage and
> alpha-to-one. Also make it possible to query for supported sample count
> with is_msaa_supported().
>
> Use explicit resource_resolve() to resolve a resource. Note that it is illegal
> to bind a unresolved resource as a sampler view, must be resolved first (as per
> d3d10 and OGL APIs, binding unresolved resource would mean that special texture
> fetch functions need to be used which give explicit control over what samples
> to fetch, which isn't supported yet).
>
> Also change surface_fill() and surface_copy() to operate directly on resources.
> Blits should operate directly on resources, most often state trackers just used
> get_tex_surface() then did a blit. Note this also means the blit bind flags are
> gone, if a driver implements this functionality it is expected to handle it for
> all resources having depth_stencil/render_target/sampler_view bind flags (might
> even require it for all bind flags?).
>
> Might want to introduce quality levels for MSAA later.
> Might need to revisit this for hw which does instant resolve.
>
> ---
>
> src/gallium/auxiliary/cso_cache/cso_context.c | 11 +++++
> src/gallium/auxiliary/cso_cache/cso_context.h | 2 +
> src/gallium/docs/source/context.rst | 16 +++++--
> src/gallium/docs/source/screen.rst | 13 +++++-
> src/gallium/include/pipe/p_context.h | 51 +++++++++++++++++-------
> src/gallium/include/pipe/p_defines.h | 2 -
> src/gallium/include/pipe/p_screen.h | 11 +++++-
> src/gallium/include/pipe/p_state.h | 2 +
> 8 files changed, 82 insertions(+), 26 deletions(-)
>
> diff --git a/src/gallium/auxiliary/cso_cache/cso_context.c b/src/gallium/auxiliary/cso_cache/cso_context.c
> index 6d0b420..50736f0 100644
> --- a/src/gallium/auxiliary/cso_cache/cso_context.c
> +++ b/src/gallium/auxiliary/cso_cache/cso_context.c
> @@ -98,6 +98,7 @@ struct cso_context {
> struct pipe_framebuffer_state fb, fb_saved;
> struct pipe_viewport_state vp, vp_saved;
> struct pipe_blend_color blend_color;
> + unsigned sample_mask sample_mask;
This doesn't look correct. sample_mask appears twice.
> struct pipe_stencil_ref stencil_ref, stencil_ref_saved;
> };
>
> @@ -984,6 +985,16 @@ enum pipe_error cso_set_blend_color(struct cso_context *ctx,
> return PIPE_OK;
> }
>
> +enum pipe_error cso_set_sample_mask(struct cso_context *ctx,
> + unsigned sample_mask)
> +{
> + if (ctx->sample_mask != sample_mask) {
> + ctx->sample_mask = sample_mask;
> + ctx->pipe->set_sample_mask(ctx->pipe, sample_mask);
> + }
> + return PIPE_OK;
> +}
> +
> enum pipe_error cso_set_stencil_ref(struct cso_context *ctx,
> const struct pipe_stencil_ref *sr)
> {
> diff --git a/src/gallium/auxiliary/cso_cache/cso_context.h b/src/gallium/auxiliary/cso_cache/cso_context.h
> index d6bcb1f..f0b07f7 100644
> --- a/src/gallium/auxiliary/cso_cache/cso_context.h
> +++ b/src/gallium/auxiliary/cso_cache/cso_context.h
> @@ -159,6 +159,8 @@ void cso_restore_viewport(struct cso_context *cso);
> enum pipe_error cso_set_blend_color(struct cso_context *cso,
> const struct pipe_blend_color *bc);
>
> +enum pipe_error cso_set_sample_mask(struct cso_context *cso,
> + unsigned stencil_mask);
>
> enum pipe_error cso_set_stencil_ref(struct cso_context *cso,
> const struct pipe_stencil_ref *sr);
> diff --git a/src/gallium/docs/source/context.rst b/src/gallium/docs/source/context.rst
> index c82e681..374711b 100644
> --- a/src/gallium/docs/source/context.rst
> +++ b/src/gallium/docs/source/context.rst
> @@ -54,6 +54,7 @@ objects. They all follow simple, one-method binding calls, e.g.
> * ``set_stencil_ref`` sets the stencil front and back reference values
> which are used as comparison values in stencil test.
> * ``set_blend_color``
> +* ``set_sample_mask``
> * ``set_clip_state``
> * ``set_polygon_stipple``
> * ``set_scissor_state`` sets the bounds for the scissor test, which culls
> @@ -255,15 +256,20 @@ Blitting
> These methods emulate classic blitter controls. They are not guaranteed to be
> available; if they are set to NULL, then they are not present.
>
> -These methods operate directly on ``pipe_surface`` objects, and stand
> +These methods operate directly on ``pipe_resource`` objects, and stand
> apart from any 3D state in the context. Blitting functionality may be
> moved to a separate abstraction at some point in the future.
>
> -``surface_fill`` performs a fill operation on a section of a surface.
> +``resource_fill_region`` performs a fill operation on a section of a resource.
>
> -``surface_copy`` blits a region of a surface to a region of another surface,
> -provided that both surfaces are the same format. The source and destination
> -may be the same surface, and overlapping blits are permitted.
> +``resource_copy_region`` blits a region of a subresource of a resource to a
> +region of another subresource of a resource, provided that both resources have the
> +same format. The source and destination may be the same resource, and overlapping
> +blits are permitted.
> +
> +``resource_resolve`` resolves a multisampled resource into a non-multisampled
> +one. Formats and dimensions must match. This function must be present if a driver
> +supports multisampling.
>
> The interfaces to these calls are likely to change to make it easier
> for a driver to batch multiple blits with the same source and
> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
> index c5815f8..2a8f696 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -128,9 +128,6 @@ resources might be created and handled quite differently.
> * ``PIPE_BIND_VERTEX_BUFFER``: A vertex buffer.
> * ``PIPE_BIND_INDEX_BUFFER``: An vertex index/element buffer.
> * ``PIPE_BIND_CONSTANT_BUFFER``: A buffer of shader constants.
> -* ``PIPE_BIND_BLIT_SOURCE``: A blit source, as given to surface_copy.
> -* ``PIPE_BIND_BLIT_DESTINATION``: A blit destination, as given to surface_copy
> - and surface_fill.
> * ``PIPE_BIND_TRANSFER_WRITE``: A transfer object which will be written to.
> * ``PIPE_BIND_TRANSFER_READ``: A transfer object which will be read from.
> * ``PIPE_BIND_CUSTOM``:
> @@ -223,6 +220,16 @@ Determine if a resource in the given format can be used in a specific manner.
>
> Returns TRUE if all usages can be satisfied.
>
> +is_msaa_supported
> +^^^^^^^^^^^^^^^^^
> +
> +Determine if a format supports multisampling with a given number of samples.
> +
> +**format** the resource format
> +
> +**sample_count** the number of samples. Valid query range is 2-32.
> +
> +Returns TRUE if sample_count number of samples is supported with this format.
>
> .. _resource_create:
>
> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
> index 6f47845..c385481 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -198,6 +198,9 @@ struct pipe_context {
> void (*set_stencil_ref)( struct pipe_context *,
> const struct pipe_stencil_ref * );
>
> + void (*set_sample_mask)( struct pipe_context *,
> + unsigned sample_mask );
> +
> void (*set_clip_state)( struct pipe_context *,
> const struct pipe_clip_state * );
>
> @@ -233,32 +236,50 @@ struct pipe_context {
>
>
> /**
> - * Surface functions
> + * Resource functions for blit-like functionality
> *
> * The pipe driver is allowed to set these functions to NULL, and in that
> * case, they will not be available.
> + * If a driver supports multisampling, resource_resolve must be available.
> */
> /*@{*/
>
> /**
> - * Copy a block of pixels from one surface to another.
> - * The surfaces must be of the same format.
> + * Copy a block of pixels from one resource to another.
> + * The resource must be of the same format.
> + * Resources with nr_samples > 1 are not allowed.
> */
> - void (*surface_copy)(struct pipe_context *pipe,
> - struct pipe_surface *dest,
> - unsigned destx, unsigned desty,
> - struct pipe_surface *src,
> - unsigned srcx, unsigned srcy,
> - unsigned width, unsigned height);
> + void (*resource_copy_region)(struct pipe_context *pipe,
> + struct pipe_resource *dst,
> + struct pipe_subresource subdst,
> + unsigned dstx, unsigned dsty, unsigned dstz,
> + struct pipe_resource *src,
> + struct pipe_subresource subsrc,
> + unsigned srcx, unsigned srcy, unsigned srcz,
> + unsigned width, unsigned height);
>
> /**
> - * Fill a region of a surface with a constant value.
> + * Fill a region of a resource with a constant value.
> + * Resources with nr_samples > 1 are not allowed.
> */
> - void (*surface_fill)(struct pipe_context *pipe,
> - struct pipe_surface *dst,
> - unsigned dstx, unsigned dsty,
> - unsigned width, unsigned height,
> - unsigned value);
> + void (*resource_fill_region)(struct pipe_context *pipe,
> + struct pipe_resource *dst,
> + struct pipe_subresource subdst,
> + struct pipe_box *dstbox,
> + unsigned srcx, unsigned srcy, unsigned srcz,
> + unsigned width, unsigned height,
> + unsigned value);
I think that once you're done with this change we should remove
surface_fill/resource_fill_region(), as no state tracker uses it -- only
drivers internally, and the 32bit value is hopeless for formats more
than 32bits.
> +
> + /**
> + * Resolve a multisampled resource into a non-multisampled one.
> + * Source and destination must have the same size and same format.
> + */
> + void (*resource_resolve)(struct pipe_context *pipe,
> + struct pipe_resource *dst,
> + struct pipe_subresource subdst,
> + struct pipe_resource *src,
> + struct pipe_subresource subsrc);
> +
> /*@}*/
>
> /**
> diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h
> index 48edfbf..3223e8d 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -284,8 +284,6 @@ enum pipe_transfer_usage {
> #define PIPE_BIND_VERTEX_BUFFER (1 << 3) /* set_vertex_buffers */
> #define PIPE_BIND_INDEX_BUFFER (1 << 4) /* draw_elements */
> #define PIPE_BIND_CONSTANT_BUFFER (1 << 5) /* set_constant_buffer */
> -#define PIPE_BIND_BLIT_SOURCE (1 << 6) /* surface_copy */
> -#define PIPE_BIND_BLIT_DESTINATION (1 << 7) /* surface_copy, fill */
> #define PIPE_BIND_DISPLAY_TARGET (1 << 8) /* flush_front_buffer */
> #define PIPE_BIND_TRANSFER_WRITE (1 << 9) /* get_transfer */
> #define PIPE_BIND_TRANSFER_READ (1 << 10) /* get_transfer */
> diff --git a/src/gallium/include/pipe/p_screen.h b/src/gallium/include/pipe/p_screen.h
> index beff1ae..1bad045 100644
> --- a/src/gallium/include/pipe/p_screen.h
> +++ b/src/gallium/include/pipe/p_screen.h
> @@ -99,10 +99,19 @@ struct pipe_screen {
> boolean (*is_format_supported)( struct pipe_screen *,
> enum pipe_format format,
> enum pipe_texture_target target,
> - unsigned bindings,
> + unsigned bindings,
> unsigned geom_flags );
>
> /**
> + * Check if the given pipe_format is supported with a requested
> + * number of samples for msaa.
> + * \param sample_count number of samples for multisampling
> + */
> + boolean (*is_msaa_supported)( struct pipe_screen *,
> + enum pipe_format format,
> + unsigned sample_count );
Instead of a new is_msaa_support() I'd prefer see sample_count in
is_format_supported or better, replace both with is_resource_supported
which takes a resource template. But I understand that's a bit beyond
the scope of this change.
> + /**
> * Create a new texture object, using the given template info.
> */
> struct pipe_resource * (*resource_create)(struct pipe_screen *,
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index a504757..f9ad07d 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -218,6 +218,8 @@ struct pipe_blend_state
> unsigned logicop_enable:1;
> unsigned logicop_func:4; /**< PIPE_LOGICOP_x */
> unsigned dither:1;
> + unsigned alpha_to_coverage:1;
> + unsigned alpha_to_one:1;
> struct pipe_rt_blend_state rt[PIPE_MAX_COLOR_BUFS];
> };
>
>
> _______________________________________________
> mesa-commit mailing list
> mesa-commit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-commit
More information about the mesa-dev
mailing list