[Mesa-dev] [PATCH 2/2] gallium/draw: add limits to the clip and cull distances
Christoph Bumiller
e0425955 at student.tuwien.ac.at
Wed Jun 12 11:15:56 PDT 2013
On 12.06.2013 15:57, Jose Fonseca wrote:
>
>
> ----- Original Message -----
>> Am 11.06.2013 05:39, schrieb Zack Rusin:
>>> There are strict limits on those registers. Define the maximums
>>> and use them instead of magic numbers. Also allows us to add
>>> some extra sanity checks.
>>> Suggested by Brian.
>>>
>>> Signed-off-by: Zack Rusin <zackr at vmware.com>
>>> ---
>>> src/gallium/auxiliary/draw/draw_context.c | 2 ++
>>> src/gallium/auxiliary/draw/draw_gs.c | 10 +++++-----
>>> src/gallium/auxiliary/draw/draw_gs.h | 4 ++--
>>> src/gallium/auxiliary/draw/draw_vs.c | 10 +++++-----
>>> src/gallium/auxiliary/draw/draw_vs.h | 4 ++--
>>> src/gallium/docs/source/tgsi.rst | 23 +++++++++++++++++++++++
>>> src/gallium/include/pipe/p_state.h | 2 ++
>>> 7 files changed, 41 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/draw/draw_context.c
>>> b/src/gallium/auxiliary/draw/draw_context.c
>>> index 0dbddb4..22c0e9b 100644
>>> --- a/src/gallium/auxiliary/draw/draw_context.c
>>> +++ b/src/gallium/auxiliary/draw/draw_context.c
>>> @@ -738,6 +738,7 @@ draw_current_shader_clipvertex_output(const struct
>>> draw_context *draw)
>>> uint
>>> draw_current_shader_clipdistance_output(const struct draw_context *draw,
>>> int index)
>>> {
>>> + debug_assert(index < PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> if (draw->gs.geometry_shader)
>>> return draw->gs.geometry_shader->clipdistance_output[index];
>>> return draw->vs.clipdistance_output[index];
>>> @@ -756,6 +757,7 @@ draw_current_shader_num_written_clipdistances(const
>>> struct draw_context *draw)
>>> uint
>>> draw_current_shader_culldistance_output(const struct draw_context *draw,
>>> int index)
>>> {
>>> + debug_assert(index < PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> if (draw->gs.geometry_shader)
>>> return draw->gs.geometry_shader->culldistance_output[index];
>>> return draw->vs.vertex_shader->culldistance_output[index];
>>> diff --git a/src/gallium/auxiliary/draw/draw_gs.c
>>> b/src/gallium/auxiliary/draw/draw_gs.c
>>> index b762dd6..cd63e2b 100644
>>> --- a/src/gallium/auxiliary/draw/draw_gs.c
>>> +++ b/src/gallium/auxiliary/draw/draw_gs.c
>>> @@ -792,13 +792,13 @@ draw_create_geometry_shader(struct draw_context
>>> *draw,
>>> if (gs->info.output_semantic_name[i] ==
>>> TGSI_SEMANTIC_VIEWPORT_INDEX)
>>> gs->viewport_index_output = i;
>>> if (gs->info.output_semantic_name[i] == TGSI_SEMANTIC_CLIPDIST) {
>>> - if (gs->info.output_semantic_index[i] == 0)
>>> - gs->clipdistance_output[0] = i;
>>> - else
>>> - gs->clipdistance_output[1] = i;
>>> + debug_assert(gs->info.output_semantic_index[i] <
>>> + PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> + gs->clipdistance_output[gs->info.output_semantic_index[i]] = i;
>>> }
>>> if (gs->info.output_semantic_name[i] == TGSI_SEMANTIC_CULLDIST) {
>>> - debug_assert(gs->info.output_semantic_index[i] <
>>> Elements(gs->culldistance_output));
>>> + debug_assert(gs->info.output_semantic_index[i] <
>>> + PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> gs->culldistance_output[gs->info.output_semantic_index[i]] = i;
>>> }
>>> }
>>> diff --git a/src/gallium/auxiliary/draw/draw_gs.h
>>> b/src/gallium/auxiliary/draw/draw_gs.h
>>> index 05d666d..e279a80 100644
>>> --- a/src/gallium/auxiliary/draw/draw_gs.h
>>> +++ b/src/gallium/auxiliary/draw/draw_gs.h
>>> @@ -67,8 +67,8 @@ struct draw_geometry_shader {
>>> struct tgsi_shader_info info;
>>> unsigned position_output;
>>> unsigned viewport_index_output;
>>> - unsigned clipdistance_output[2];
>>> - unsigned culldistance_output[2];
>>> + unsigned
>>> clipdistance_output[PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT];
>>> + unsigned
>>> culldistance_output[PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT];
>>>
>>> unsigned max_output_vertices;
>>> unsigned primitive_boundary;
>>> diff --git a/src/gallium/auxiliary/draw/draw_vs.c
>>> b/src/gallium/auxiliary/draw/draw_vs.c
>>> index a0bebcc..bbccbe4 100644
>>> --- a/src/gallium/auxiliary/draw/draw_vs.c
>>> +++ b/src/gallium/auxiliary/draw/draw_vs.c
>>> @@ -86,12 +86,12 @@ draw_create_vertex_shader(struct draw_context *draw,
>>> found_clipvertex = TRUE;
>>> vs->clipvertex_output = i;
>>> } else if (vs->info.output_semantic_name[i] ==
>>> TGSI_SEMANTIC_CLIPDIST) {
>>> - if (vs->info.output_semantic_index[i] == 0)
>>> - vs->clipdistance_output[0] = i;
>>> - else
>>> - vs->clipdistance_output[1] = i;
>>> + debug_assert(vs->info.output_semantic_index[i] <
>>> + PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> + vs->clipdistance_output[vs->info.output_semantic_index[i]] =
>>> i;
>>> } else if (vs->info.output_semantic_name[i] ==
>>> TGSI_SEMANTIC_CULLDIST) {
>>> - debug_assert(vs->info.output_semantic_index[i] <
>>> Elements(vs->culldistance_output));
>>> + debug_assert(vs->info.output_semantic_index[i] <
>>> + PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT);
>>> vs->culldistance_output[vs->info.output_semantic_index[i]] =
>>> i;
>>> }
>>> }
>>> diff --git a/src/gallium/auxiliary/draw/draw_vs.h
>>> b/src/gallium/auxiliary/draw/draw_vs.h
>>> index 7635aba..425efa3 100644
>>> --- a/src/gallium/auxiliary/draw/draw_vs.h
>>> +++ b/src/gallium/auxiliary/draw/draw_vs.h
>>> @@ -112,8 +112,8 @@ struct draw_vertex_shader {
>>> unsigned position_output;
>>> unsigned edgeflag_output;
>>> unsigned clipvertex_output;
>>> - unsigned clipdistance_output[2];
>>> - unsigned culldistance_output[2];
>>> + unsigned
>>> clipdistance_output[PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT];
>>> + unsigned
>>> culldistance_output[PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT];
>>> /* Extracted from shader:
>>> */
>>> const float (*immediates)[4];
>>> diff --git a/src/gallium/docs/source/tgsi.rst
>>> b/src/gallium/docs/source/tgsi.rst
>>> index f3aebb3..3f48b51 100644
>>> --- a/src/gallium/docs/source/tgsi.rst
>>> +++ b/src/gallium/docs/source/tgsi.rst
>>> @@ -2435,6 +2435,29 @@ float32 signed distance to a plane. Primitives will
>>> be completely
>>> discarded if the plane distance for all of the vertices in the
>>> primitive are < 0. If a vertex has a cull distance of NaN, that
>>> vertex counts as "out" (as if its < 0);
>>> +The limits on both clip and cull distances are bound
>>> +by the PIPE_MAX_CLIP_OR_CULL_DISTANCE_COUNT define which defines
>>> +the maximum number of components that can be used to hold the
>>> +distances and by the PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT
>>> +which specifies the maximum number of registers which can be
>>> +annotated with those semantics.
>>> +
>>> +
>>> +TGSI_SEMANTIC_CLIPDIST
>>> +""""""""""""""""""""""
>>> +
>>> +When components of vertex elements are identified this way, these
>>> +values are each assumed to be a float32 signed distance to a plane.
>>> +Primitive setup only invokes rasterization on pixels for which
>>> +the interpolated plane distances are >= 0. Multiple clip planes
>>> +can be implemented simultaneously, by annotating multiple
>>> +components of one or more vertex elements with the above specified
>>> +semantic. The limits on both clip and cull distances are bound
>>> +by the PIPE_MAX_CLIP_OR_CULL_DISTANCE_COUNT define which defines
>>> +the maximum number of components that can be used to hold the
>>> +distances and by the PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT
>>> +which specifies the maximum number of registers which can be
>>> +annotated with those semantics.
>>>
>>>
>>> Declaration Interpolate
>>> diff --git a/src/gallium/include/pipe/p_state.h
>>> b/src/gallium/include/pipe/p_state.h
>>> index ff0aac7..29e8cc9 100644
>>> --- a/src/gallium/include/pipe/p_state.h
>>> +++ b/src/gallium/include/pipe/p_state.h
>>> @@ -66,6 +66,8 @@ extern "C" {
>>> #define PIPE_MAX_SO_BUFFERS 4
>>> #define PIPE_MAX_SO_OUTPUTS 64
>>> #define PIPE_MAX_VIEWPORTS 16
>>> +#define PIPE_MAX_CLIP_OR_CULL_DISTANCE_COUNT 8
>>> +#define PIPE_MAX_CLIP_OR_CULL_DISTANCE_ELEMENT_COUNT 2
>>>
Isn't it a bit ambiguous to call it "ELEMENT" count ? Wouldn't you
equate a clip distance element to a vector element, i.e. a component ?
At least I was a bit confused at first, maybe you were thinking more
along the lines of a vertex element. Making it "VEC4_COUNT" or
"REGISTER_COUNT" would be better I think.
On an unrelated note ... packing clip distances into vec4s sucks, I want
scalar arrays.
>>>
>>> struct pipe_reference
>>>
>>
>> Series looks good to me.
>
> Same here.
>
> Jose
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list