[Mesa-dev] [PATCH] gallium: new, unified pipe_context::set_sampler_views() function

Emil Velikov emil.l.velikov at gmail.com
Tue Oct 15 19:23:15 PDT 2013


On 08/10/13 01:23, Brian Paul wrote:
> The new function replaces four old functions: set_fragment/vertex/
> geometry/compute_sampler_views().
> 
> Note: at this time, it's expected that the 'start' parameter will
> always be zero.
> 
> ---
> 

Hello Brian,

A couple of trivial questions

Would you mind dropping the default case or adding an assert/debug_error
in the PIPE_SHADER switch for each driver?

   switch (shader_stage) {
   case PIPE_SHADER_FRAGMENT:
      ...
   case PIPE_SHADER_VERTEX:
      ...
   case PIPE_SHADER_GEOMETRY:
      ...
   default:
      ;
   }

This way issues can spotted easier (compiler warnings or assert).

With the introduction of "start", the compiler will produce a few
warnings about "unused variable start". Maybe use
    (void) start;

after each assert(start == 0); to silence them ?

> This change touches quite a few files.  I've probably missed
> something in drivers or state trackers that I can't test.
> Please test if you're able.  Thanks.
> ---
Will run a quick piglit with and w/o on my nv50 this weekend.

>  src/gallium/auxiliary/cso_cache/cso_context.c     |   36 +++---------
>  src/gallium/auxiliary/draw/draw_pipe_aaline.c     |   37 ++++++------
>  src/gallium/auxiliary/draw/draw_pipe_pstipple.c   |   33 ++++++-----
>  src/gallium/auxiliary/util/u_blitter.c            |   12 ++--
>  src/gallium/auxiliary/vl/vl_compositor.c          |    4 +-
>  src/gallium/auxiliary/vl/vl_idct.c                |    6 +-
>  src/gallium/auxiliary/vl/vl_matrix_filter.c       |    3 +-
>  src/gallium/auxiliary/vl/vl_mc.c                  |    3 +-
>  src/gallium/auxiliary/vl/vl_median_filter.c       |    3 +-
>  src/gallium/auxiliary/vl/vl_mpeg12_decoder.c      |    4 +-
>  src/gallium/auxiliary/vl/vl_zscan.c               |    3 +-
>  src/gallium/docs/d3d11ddi.txt                     |    4 +-
>  src/gallium/docs/source/context.rst               |   14 ++---
>  src/gallium/drivers/freedreno/freedreno_texture.c |   22 +++++++-
>  src/gallium/drivers/galahad/glhd_context.c        |   57 ++-----------------
>  src/gallium/drivers/i915/i915_state.c             |   22 +++++++-
>  src/gallium/drivers/identity/id_context.c         |   47 ++--------------
>  src/gallium/drivers/ilo/ilo_state.c               |   27 +++++++--
>  src/gallium/drivers/llvmpipe/lp_state_sampler.c   |   30 +---------
>  src/gallium/drivers/noop/noop_state.c             |   13 ++---
>  src/gallium/drivers/nouveau/nv30/nv30_context.h   |    8 +++
>  src/gallium/drivers/nouveau/nv30/nv30_fragtex.c   |   24 +++++++-
>  src/gallium/drivers/nouveau/nv30/nv40_verttex.c   |    4 +-
>  src/gallium/drivers/nouveau/nv50/nv50_state.c     |   39 ++++++-------
>  src/gallium/drivers/nouveau/nvc0/nvc0_state.c     |   57 ++++++++-----------
>  src/gallium/drivers/r300/r300_state.c             |   13 +++--
>  src/gallium/drivers/r600/evergreen_compute.c      |    3 +-
>  src/gallium/drivers/r600/r600_pipe.h              |    5 ++
>  src/gallium/drivers/r600/r600_state_common.c      |   20 ++-----
>  src/gallium/drivers/radeonsi/si_state.c           |   30 ++++------
>  src/gallium/drivers/rbug/rbug_context.c           |   30 +---------
>  src/gallium/drivers/softpipe/sp_state_sampler.c   |   32 +----------
>  src/gallium/drivers/svga/svga_pipe_sampler.c      |   11 +---
>  src/gallium/drivers/trace/tr_context.c            |   62 ++-------------------
>  src/gallium/include/pipe/p_context.h              |   18 +-----
>  src/gallium/tests/graw/fs-test.c                  |    2 +-
>  src/gallium/tests/graw/gs-test.c                  |    2 +-
>  src/gallium/tests/graw/quad-sample.c              |    2 +-
>  src/gallium/tests/graw/quad-tex.c                 |    2 +-
>  src/gallium/tests/graw/tex-srgb.c                 |    4 +-
>  src/gallium/tests/graw/tex-swizzle.c              |    2 +-
>  src/gallium/tests/graw/vs-test.c                  |    2 +-
>  src/gallium/tests/trivial/compute.c               |    4 +-
>  src/gallium/tools/trace/dump_state.py             |   14 +----
>  44 files changed, 278 insertions(+), 492 deletions(-)
> 
[...]
> diff --git a/src/gallium/auxiliary/vl/vl_zscan.c b/src/gallium/auxiliary/vl/vl_zscan.c
> index 2d616d57..40502ae 100644
> --- a/src/gallium/auxiliary/vl/vl_zscan.c
> +++ b/src/gallium/auxiliary/vl/vl_zscan.c
> @@ -578,7 +578,8 @@ vl_zscan_render(struct vl_zscan *zscan, struct vl_zscan_buffer *buffer, unsigned
>                                      0, 3, zscan->samplers);
>     zscan->pipe->set_framebuffer_state(zscan->pipe, &buffer->fb_state);
>     zscan->pipe->set_viewport_states(zscan->pipe, 0, 1, &buffer->viewport);
> -   zscan->pipe->set_fragment_sampler_views(zscan->pipe, 3, &buffer->src);
> +   zscan->pipe->set_sampler_views(zscan->pipe, PIPE_SHADER_FRAGMENT,
> +                                  0, 3, &buffer->src);
>     zscan->pipe->bind_vs_state(zscan->pipe, zscan->vs);
>     zscan->pipe->bind_fs_state(zscan->pipe, zscan->fs);
>     util_draw_arrays_instanced(zscan->pipe, PIPE_PRIM_QUADS, 0, 4, 0, num_instances);
> diff --git a/src/gallium/docs/d3d11ddi.txt b/src/gallium/docs/d3d11ddi.txt
> index 14a589f..a703648 100644
> --- a/src/gallium/docs/d3d11ddi.txt
> +++ b/src/gallium/docs/d3d11ddi.txt
> @@ -343,7 +343,7 @@ PsSetShader -> bind_fs_state
>  PsSetShaderWithIfaces (D3D11 only)
>  	- Gallium does not support shader interfaces
>  
> -PsSetShaderResources -> set_fragment_sampler_views
> +PsSetShaderResources -> set_sampler_views
>  	* may want to allow binding subsets instead of all at once
I believe that this patch addresses the above comment. Or is it simply a
preliminary step towards it ?

>  
>  QueryBegin -> begin_query
> @@ -458,5 +458,5 @@ VsSetShader -> bind_vs_state
>  VsSetShaderWithIfaces (D3D11 only)
>  	- Gallium does not support shader interfaces
>  
> -VsSetShaderResources  -> set_fragment_sampler_views
> +VsSetShaderResources  -> set_sampler_views
>  	* may want to allow binding subsets instead of all at once
Ditto

Just noticed that bind_*_sampler_views are still mentioned in the
document. I'm assuming that it's a leftover from the bind_sampler_views
series, is that right ?

Cheers
Emil


More information about the mesa-dev mailing list