Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
On 05/23/2013 03:02 PM, Roland Scheidegger wrote:
Am 23.05.2013 22:33, schrieb Zack Rusin:
Gallium supported only a single viewport/scissor combination. This
commit changes the interface to allow us to add support for multiple
viewports/scissors.
Signed-off-by: Zack Rusin
---
diff --git a/src/gallium/include/pipe/p_context.h
b/src/gallium/include/pipe/p_context.h
index d1130bc..eaaa043 100644
--- a/src/gallium/include/pipe/p_context.h
+++ b/src/gallium/include/pipe/p_context.h
@@ -211,11 +211,13 @@ struct pipe_context {
void (*set_polygon_stipple)( struct pipe_context *,
const struct pipe_poly_stipple * );
- void (*set_scissor_state)( struct pipe_context *,
- const struct pipe_scissor_state * );
+ void (*set_scissor_states)( struct pipe_context *,
+ unsigned num_scissors,
+ const struct pipe_scissor_state * );
- void (*set_viewport_state)( struct pipe_context *,
- const struct pipe_viewport_state * );
+ void (*set_viewport_states)( struct pipe_context *,
+unsigned num_viewports,
+const struct pipe_viewport_state *);
void (*set_fragment_sampler_views)(struct pipe_context *,
unsigned num_views,
diff --git a/src/gallium/include/pipe/p_defines.h
b/src/gallium/include/pipe/p_defines.h
index bb86968..00f0a37 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -507,7 +507,8 @@ enum pipe_cap {
PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER = 80,
PIPE_CAP_QUERY_PIPELINE_STATISTICS = 81,
PIPE_CAP_TEXTURE_BORDER_COLOR_QUIRK = 82,
- PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE = 83
+ PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE = 83,
+ PIPE_CAP_MULTIPLE_VIEWPORTS = 84
Would it be better if this were PIPE_CAP_MAX_VIEWPORTS instead? Though I
guess there's no real need right now to support anything but 16 (as
that's needed by d3d10/11, and is the minimum supported value for GL,
though GL would allow for more), so I don't have a strong opinion on that.
I second this suggestion.
-Brian
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
Just some minor formatting nits below... On 05/23/2013 02:33 PM, Zack Rusin wrote: Gallium supported only a single viewport/scissor combination. This commit changes the interface to allow us to add support for multiple viewports/scissors. Signed-off-by: Zack Rusin --- src/gallium/auxiliary/cso_cache/cso_context.c | 37 +++ src/gallium/auxiliary/cso_cache/cso_context.h |9 +++--- src/gallium/auxiliary/draw/draw_context.c |6 ++-- src/gallium/auxiliary/draw/draw_context.h |5 +-- src/gallium/auxiliary/hud/hud_context.c |6 ++-- src/gallium/auxiliary/postprocess/pp_run.c |6 ++-- src/gallium/auxiliary/tgsi/tgsi_scan.c |6 src/gallium/auxiliary/tgsi/tgsi_scan.h |1 + src/gallium/auxiliary/tgsi/tgsi_strings.c |3 +- src/gallium/auxiliary/util/u_blit.c | 12 src/gallium/auxiliary/util/u_blitter.c |8 ++--- src/gallium/auxiliary/util/u_gen_mipmap.c |6 ++-- src/gallium/auxiliary/vl/vl_compositor.c|4 +-- src/gallium/auxiliary/vl/vl_idct.c |4 +-- src/gallium/auxiliary/vl/vl_matrix_filter.c |2 +- src/gallium/auxiliary/vl/vl_mc.c|2 +- src/gallium/auxiliary/vl/vl_median_filter.c |2 +- src/gallium/auxiliary/vl/vl_zscan.c |2 +- src/gallium/docs/source/context.rst |8 +++-- src/gallium/drivers/freedreno/freedreno_state.c | 10 +++--- src/gallium/drivers/galahad/glhd_context.c | 16 +- src/gallium/drivers/i915/i915_state.c | 12 +--- src/gallium/drivers/identity/id_context.c | 22 -- src/gallium/drivers/ilo/ilo_state.c | 14 + src/gallium/drivers/llvmpipe/lp_screen.c|2 ++ src/gallium/drivers/llvmpipe/lp_state_clip.c| 20 ++-- src/gallium/drivers/noop/noop_state.c | 14 + src/gallium/drivers/nv30/nv30_draw.c|2 +- src/gallium/drivers/nv30/nv30_state.c | 14 + src/gallium/drivers/nv50/nv50_state.c | 16 +- src/gallium/drivers/nvc0/nvc0_state.c | 14 + src/gallium/drivers/r300/r300_context.c |2 +- src/gallium/drivers/r300/r300_state.c | 16 +- src/gallium/drivers/r600/evergreen_state.c |5 +-- src/gallium/drivers/r600/r600_state.c |7 +++-- src/gallium/drivers/r600/r600_state_common.c|9 +++--- src/gallium/drivers/radeonsi/si_state.c | 14 + src/gallium/drivers/rbug/rbug_context.c | 22 -- src/gallium/drivers/softpipe/sp_screen.c|2 ++ src/gallium/drivers/softpipe/sp_state_clip.c| 16 +- src/gallium/drivers/svga/svga_pipe_misc.c | 18 ++- src/gallium/drivers/svga/svga_swtnl_state.c |2 +- src/gallium/drivers/trace/tr_context.c | 28 + src/gallium/include/pipe/p_context.h| 10 +++--- src/gallium/include/pipe/p_defines.h|3 +- src/gallium/include/pipe/p_shader_tokens.h |3 +- src/gallium/include/pipe/p_state.h |1 + src/gallium/state_trackers/vega/renderer.c | 10 +++--- src/gallium/state_trackers/xa/xa_renderer.c |2 +- src/gallium/state_trackers/xorg/xorg_renderer.c |2 +- src/gallium/tests/graw/fs-test.c|2 +- src/gallium/tests/graw/graw_util.h |2 +- src/gallium/tests/graw/gs-test.c|2 +- src/gallium/tests/graw/quad-sample.c|2 +- src/gallium/tests/graw/shader-leak.c|2 +- src/gallium/tests/graw/tri-gs.c |2 +- src/gallium/tests/graw/tri-instanced.c |2 +- src/gallium/tests/graw/vs-test.c|2 +- src/gallium/tests/trivial/quad-tex.c|2 +- src/gallium/tests/trivial/tri.c |2 +- src/mesa/state_tracker/st_atom_scissor.c|2 +- src/mesa/state_tracker/st_atom_viewport.c |2 +- src/mesa/state_tracker/st_cb_bitmap.c |6 ++-- src/mesa/state_tracker/st_cb_clear.c|6 ++-- src/mesa/state_tracker/st_cb_drawpixels.c |6 ++-- src/mesa/state_tracker/st_cb_drawtex.c |6 ++-- src/mesa/state_tracker/st_draw_feedback.c |2 +- 67 files changed, 290 insertions(+), 217 deletions(-) diff --git a/src/gallium/drivers/galahad/glhd_context.c b/src/gallium/drivers/galahad/glhd_context.c index a73a3ad..849c12e 100644 --- a/src/gallium/drivers/galahad/glhd_context.c +++ b/src/gallium/drivers/galahad/glhd_context.c @@ -524,25 +524,27 @@ galahad_context_set_polygon_stipple(struct pipe_context *_pipe, } static void -galahad_context_set_scissor_state(struct pipe_context *_pipe, +galahad_context_set_scissor_states(s
Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
On Thu, May 23, 2013 at 11:59 PM, Zack Rusin wrote: >> 1) I prefer this interface instead: >> >> void (*set_scissor_states)( struct pipe_context *, >>unsigned start_slot, unsigned count, >>const struct pipe_scissor_state * ); >> >> void (*set_viewport_states)( struct pipe_context *, >> unsigned start_slot, unsigned count, >> const struct pipe_viewport_state *); >> >> Both function should allow updating only a subset of all viewports and >> scissors (from start_slot to start_slot+count-1). This is especially >> important for meta ops (u_gen_mipmap, etc.), which need to update only >> the first viewport (and no scissor), leaving the other viewports >> unchanged. This idea is not new: the vertex buffer and compute sampler >> functions have the start_slot parameter too. > > It's obviously based on the d3d10 interface, it just seems like a lot simpler > interface. I understand that gl does specify the start slot when updating the > viewports, but I did think that explicitly specifying viewports from 0 up to > the number of viewports set, was semantically very clear. The number of viewports set doesn't really matter. On latest hardware, there are always 16 viewports. And we usually implement the union of both GL and D3D. It would be reasonable to have the start_slot parameter for pretty much every client (OpenGL, hardware drivers, and internal Mesa meta ops) except D3D (which isn't even present in the repository). > >> 2) What does cso_context need to keep a copy of all viewports for? All >> meta ops need only one viewport, just as they need only one vertex >> buffer and one constant buffer (and cso_context doesn't really allow >> meta ops to use more than that). For example, see how the cso_context >> interface for saving and restoring the constant buffer slot 0 looks >> like. It's preferable to use the same mechanism unless there is a need >> to have the save and restore functionality for all slots. > > It's a bit weird to say "none of the utils uses multiple viewports" given > that this is the first commit that at all introduces the concept. What if > something will? It's not a lot of code and it seemed to make sense to do it > properly from the start. I can imagine u_gen_mipmap using layered rendering (a very important feature of geometry shaders), however nothing comes to mind which would need to use multiple viewports. If we ever need the support for multiple viewports in meta ops / internal rendering code in the middle application rendering, we can add the necessary code to cso_context. Until then, it's just burning CPU cycles for nothing. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
> 1) I prefer this interface instead: > > void (*set_scissor_states)( struct pipe_context *, >unsigned start_slot, unsigned count, >const struct pipe_scissor_state * ); > > void (*set_viewport_states)( struct pipe_context *, > unsigned start_slot, unsigned count, > const struct pipe_viewport_state *); > > Both function should allow updating only a subset of all viewports and > scissors (from start_slot to start_slot+count-1). This is especially > important for meta ops (u_gen_mipmap, etc.), which need to update only > the first viewport (and no scissor), leaving the other viewports > unchanged. This idea is not new: the vertex buffer and compute sampler > functions have the start_slot parameter too. It's obviously based on the d3d10 interface, it just seems like a lot simpler interface. I understand that gl does specify the start slot when updating the viewports, but I did think that explicitly specifying viewports from 0 up to the number of viewports set, was semantically very clear. > 2) What does cso_context need to keep a copy of all viewports for? All > meta ops need only one viewport, just as they need only one vertex > buffer and one constant buffer (and cso_context doesn't really allow > meta ops to use more than that). For example, see how the cso_context > interface for saving and restoring the constant buffer slot 0 looks > like. It's preferable to use the same mechanism unless there is a need > to have the save and restore functionality for all slots. It's a bit weird to say "none of the utils uses multiple viewports" given that this is the first commit that at all introduces the concept. What if something will? It's not a lot of code and it seemed to make sense to do it properly from the start. z ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
1) I prefer this interface instead: void (*set_scissor_states)( struct pipe_context *, unsigned start_slot, unsigned count, const struct pipe_scissor_state * ); void (*set_viewport_states)( struct pipe_context *, unsigned start_slot, unsigned count, const struct pipe_viewport_state *); Both function should allow updating only a subset of all viewports and scissors (from start_slot to start_slot+count-1). This is especially important for meta ops (u_gen_mipmap, etc.), which need to update only the first viewport (and no scissor), leaving the other viewports unchanged. This idea is not new: the vertex buffer and compute sampler functions have the start_slot parameter too. 2) What does cso_context need to keep a copy of all viewports for? All meta ops need only one viewport, just as they need only one vertex buffer and one constant buffer (and cso_context doesn't really allow meta ops to use more than that). For example, see how the cso_context interface for saving and restoring the constant buffer slot 0 looks like. It's preferable to use the same mechanism unless there is a need to have the save and restore functionality for all slots. Marek On Thu, May 23, 2013 at 10:33 PM, Zack Rusin wrote: > Gallium supported only a single viewport/scissor combination. This > commit changes the interface to allow us to add support for multiple > viewports/scissors. > > Signed-off-by: Zack Rusin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] gallium: Add support for multiple viewports
Am 23.05.2013 22:33, schrieb Zack Rusin:
> Gallium supported only a single viewport/scissor combination. This
> commit changes the interface to allow us to add support for multiple
> viewports/scissors.
>
> Signed-off-by: Zack Rusin
> ---
> diff --git a/src/gallium/include/pipe/p_context.h
> b/src/gallium/include/pipe/p_context.h
> index d1130bc..eaaa043 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -211,11 +211,13 @@ struct pipe_context {
> void (*set_polygon_stipple)( struct pipe_context *,
> const struct pipe_poly_stipple * );
>
> - void (*set_scissor_state)( struct pipe_context *,
> - const struct pipe_scissor_state * );
> + void (*set_scissor_states)( struct pipe_context *,
> + unsigned num_scissors,
> + const struct pipe_scissor_state * );
>
> - void (*set_viewport_state)( struct pipe_context *,
> - const struct pipe_viewport_state * );
> + void (*set_viewport_states)( struct pipe_context *,
> +unsigned num_viewports,
> +const struct pipe_viewport_state *);
>
> void (*set_fragment_sampler_views)(struct pipe_context *,
>unsigned num_views,
> diff --git a/src/gallium/include/pipe/p_defines.h
> b/src/gallium/include/pipe/p_defines.h
> index bb86968..00f0a37 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -507,7 +507,8 @@ enum pipe_cap {
> PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER = 80,
> PIPE_CAP_QUERY_PIPELINE_STATISTICS = 81,
> PIPE_CAP_TEXTURE_BORDER_COLOR_QUIRK = 82,
> - PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE = 83
> + PIPE_CAP_MAX_TEXTURE_BUFFER_SIZE = 83,
> + PIPE_CAP_MULTIPLE_VIEWPORTS = 84
Would it be better if this were PIPE_CAP_MAX_VIEWPORTS instead? Though I
guess there's no real need right now to support anything but 16 (as
that's needed by d3d10/11, and is the minimum supported value for GL,
though GL would allow for more), so I don't have a strong opinion on that.
Also please document that CAP bit.
> };
>
> #define PIPE_QUIRK_TEXTURE_BORDER_COLOR_SWIZZLE_NV50 (1 << 0)
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
> b/src/gallium/include/pipe/p_shader_tokens.h
> index 50de2d3..b33cf1d 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -164,7 +164,8 @@ struct tgsi_declaration_interp
> #define TGSI_SEMANTIC_THREAD_ID 18 /**< block-relative id of the current
> thread */
> #define TGSI_SEMANTIC_TEXCOORD 19 /**< texture or sprite coordinates */
> #define TGSI_SEMANTIC_PCOORD 20 /**< point sprite coordinate */
> -#define TGSI_SEMANTIC_COUNT 21 /**< number of semantic values */
> +#define TGSI_SEMANTIC_VIEWPORT_INDEX 21 /**< viewport index */
> +#define TGSI_SEMANTIC_COUNT 22 /**< number of semantic values */
>
> struct tgsi_declaration_semantic
> {
> diff --git a/src/gallium/include/pipe/p_state.h
> b/src/gallium/include/pipe/p_state.h
> index 262078d..ff0aac7 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -65,6 +65,7 @@ extern "C" {
> #define PIPE_MAX_TEXTURE_LEVELS 16
> #define PIPE_MAX_SO_BUFFERS4
> #define PIPE_MAX_SO_OUTPUTS 64
> +#define PIPE_MAX_VIEWPORTS16
>
>
> struct pipe_reference
Otherwise looks good to me.
Roland
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
