[Mesa-dev] [PATCH 01/10] gallium: cleanup set_tess_state

Nicolai Hähnle nhaehnle at gmail.com
Tue Jun 14 16:30:09 UTC 2016


On 14.06.2016 18:02, Ilia Mirkin wrote:
> Can you explain the motivation behind this change? I'm adding a
> ->set_window_rectangles thing which also takes multiple parameters.
> What's the advantage of stuffing things into a struct first?

FWIW, I tend to be mildly supportive of changes like this. At least, the 
other extreme where functions grow multiple bool or int parameters over 
time is much worse. But in this particular case, changing this around 
might be too eager.

Perhaps teaching the script to deal with slightly more complicated cases 
will help elsewhere, too.

Nicolai

>
>    -ilia
>
> On Tue, Jun 14, 2016 at 11:57 AM, Rob Clark <robdclark at gmail.com> wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> The reset of the state APIs take state structs, rather than inline
>> parameters (with the exception of a couple which just amount to a single
>> uint).
>>
>> This makes the API more regular and simplifies autogeneration of the
>> gallium state related APIs.
>>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> ---
>>   src/gallium/drivers/ddebug/dd_context.c       |  9 ++++-----
>>   src/gallium/drivers/nouveau/nvc0/nvc0_state.c |  7 +++----
>>   src/gallium/drivers/r600/evergreen_state.c    |  7 +++----
>>   src/gallium/drivers/radeonsi/si_state.c       |  7 +++----
>>   src/gallium/drivers/trace/tr_context.c        |  9 ++++-----
>>   src/gallium/include/pipe/p_context.h          |  4 ++--
>>   src/gallium/include/pipe/p_state.h            |  8 ++++++++
>>   src/mesa/state_tracker/st_atom_tess.c         | 13 ++++++++++---
>>   8 files changed, 37 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/gallium/drivers/ddebug/dd_context.c b/src/gallium/drivers/ddebug/dd_context.c
>> index 0f8ef18..06b7c91 100644
>> --- a/src/gallium/drivers/ddebug/dd_context.c
>> +++ b/src/gallium/drivers/ddebug/dd_context.c
>> @@ -380,15 +380,14 @@ dd_context_set_viewport_states(struct pipe_context *_pipe,
>>   }
>>
>>   static void dd_context_set_tess_state(struct pipe_context *_pipe,
>> -                                      const float default_outer_level[4],
>> -                                      const float default_inner_level[2])
>> +                                      const struct pipe_tess_state *state)
>>   {
>>      struct dd_context *dctx = dd_context(_pipe);
>>      struct pipe_context *pipe = dctx->pipe;
>>
>> -   memcpy(dctx->tess_default_levels, default_outer_level, sizeof(float) * 4);
>> -   memcpy(dctx->tess_default_levels+4, default_inner_level, sizeof(float) * 2);
>> -   pipe->set_tess_state(pipe, default_outer_level, default_inner_level);
>> +   memcpy(dctx->tess_default_levels, state->default_outer_level, sizeof(float) * 4);
>> +   memcpy(dctx->tess_default_levels+4, state->default_inner_level, sizeof(float) * 2);
>> +   pipe->set_tess_state(pipe, state);
>>   }
>>
>>
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> index 92161ec..a9c1830 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
>> @@ -1001,13 +1001,12 @@ nvc0_set_viewport_states(struct pipe_context *pipe,
>>
>>   static void
>>   nvc0_set_tess_state(struct pipe_context *pipe,
>> -                    const float default_tess_outer[4],
>> -                    const float default_tess_inner[2])
>> +                    const struct pipe_tess_state *state)
>>   {
>>      struct nvc0_context *nvc0 = nvc0_context(pipe);
>>
>> -   memcpy(nvc0->default_tess_outer, default_tess_outer, 4 * sizeof(float));
>> -   memcpy(nvc0->default_tess_inner, default_tess_inner, 2 * sizeof(float));
>> +   memcpy(nvc0->default_tess_outer, state->default_tess_outer, 4 * sizeof(float));
>> +   memcpy(nvc0->default_tess_inner, state->default_tess_inner, 2 * sizeof(float));
>>      nvc0->dirty_3d |= NVC0_NEW_3D_TESSFACTOR;
>>   }
>>
>> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
>> index 1ac8914..2a424f5 100644
>> --- a/src/gallium/drivers/r600/evergreen_state.c
>> +++ b/src/gallium/drivers/r600/evergreen_state.c
>> @@ -3569,13 +3569,12 @@ fallback:
>>   }
>>
>>   static void evergreen_set_tess_state(struct pipe_context *ctx,
>> -                                    const float default_outer_level[4],
>> -                                    const float default_inner_level[2])
>> +                                    const struct pipe_tess_state *state)
>>   {
>>          struct r600_context *rctx = (struct r600_context *)ctx;
>>
>> -       memcpy(rctx->tess_state, default_outer_level, sizeof(float) * 4);
>> -       memcpy(rctx->tess_state+4, default_inner_level, sizeof(float) * 2);
>> +       memcpy(rctx->tess_state, state->default_outer_level, sizeof(float) * 4);
>> +       memcpy(rctx->tess_state+4, state->default_inner_level, sizeof(float) * 2);
>>          rctx->tess_state_dirty = true;
>>   }
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c
>> index 0c52eee..6ef3fe5 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -3238,15 +3238,14 @@ static void si_set_index_buffer(struct pipe_context *ctx,
>>    */
>>
>>   static void si_set_tess_state(struct pipe_context *ctx,
>> -                             const float default_outer_level[4],
>> -                             const float default_inner_level[2])
>> +                             const struct pipe_tess_state *state)
>>   {
>>          struct si_context *sctx = (struct si_context *)ctx;
>>          struct pipe_constant_buffer cb;
>>          float array[8];
>>
>> -       memcpy(array, default_outer_level, sizeof(float) * 4);
>> -       memcpy(array+4, default_inner_level, sizeof(float) * 2);
>> +       memcpy(array, state->default_outer_level, sizeof(float) * 4);
>> +       memcpy(array+4, state->default_inner_level, sizeof(float) * 2);
>>
>>          cb.buffer = NULL;
>>          cb.user_buffer = NULL;
>> diff --git a/src/gallium/drivers/trace/tr_context.c b/src/gallium/drivers/trace/tr_context.c
>> index 18c5c43..0dd07e9 100644
>> --- a/src/gallium/drivers/trace/tr_context.c
>> +++ b/src/gallium/drivers/trace/tr_context.c
>> @@ -1652,19 +1652,18 @@ trace_context_memory_barrier(struct pipe_context *_context,
>>
>>   static void
>>   trace_context_set_tess_state(struct pipe_context *_context,
>> -                             const float default_outer_level[4],
>> -                             const float default_inner_level[2])
>> +                             const struct pipe_tess_state *state)
>>   {
>>      struct trace_context *tr_context = trace_context(_context);
>>      struct pipe_context *context = tr_context->pipe;
>>
>>      trace_dump_call_begin("pipe_context", "set_tess_state");
>>      trace_dump_arg(ptr, context);
>> -   trace_dump_arg_array(float, default_outer_level, 4);
>> -   trace_dump_arg_array(float, default_inner_level, 2);
>> +   trace_dump_arg_array(float, state->default_outer_level, 4);
>> +   trace_dump_arg_array(float, state->default_inner_level, 2);
>>      trace_dump_call_end();
>>
>> -   context->set_tess_state(context, default_outer_level, default_inner_level);
>> +   context->set_tess_state(context, state);
>>   }
>>
>>
>> diff --git a/src/gallium/include/pipe/p_context.h b/src/gallium/include/pipe/p_context.h
>> index 9d7a8eb..9eb4a87 100644
>> --- a/src/gallium/include/pipe/p_context.h
>> +++ b/src/gallium/include/pipe/p_context.h
>> @@ -60,6 +60,7 @@ struct pipe_resolve_info;
>>   struct pipe_resource;
>>   struct pipe_sampler_state;
>>   struct pipe_sampler_view;
>> +struct pipe_tess_state;
>>   struct pipe_scissor_state;
>>   struct pipe_shader_buffer;
>>   struct pipe_shader_state;
>> @@ -284,8 +285,7 @@ struct pipe_context {
>>                                struct pipe_sampler_view **);
>>
>>      void (*set_tess_state)(struct pipe_context *,
>> -                          const float default_outer_level[4],
>> -                          const float default_inner_level[2]);
>> +                          const struct pipe_tess_state *);
>>
>>      /**
>>       * Sets the debug callback. If the pointer is null, then no callback is
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index 396f563..f2ebc53 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -453,6 +453,14 @@ struct pipe_image_view
>>      } u;
>>   };
>>
>> +/**
>> + * Tessellation state.
>> + */
>> +struct pipe_tess_state
>> +{
>> +   float default_outer_level[4];
>> +   float default_inner_level[2];
>> +};
>>
>>   /**
>>    * Subregion of 1D/2D/3D image resource.
>> diff --git a/src/mesa/state_tracker/st_atom_tess.c b/src/mesa/state_tracker/st_atom_tess.c
>> index 8e6287a..61aa92f 100644
>> --- a/src/mesa/state_tracker/st_atom_tess.c
>> +++ b/src/mesa/state_tracker/st_atom_tess.c
>> @@ -34,6 +34,7 @@
>>   #include "main/macros.h"
>>   #include "st_context.h"
>>   #include "pipe/p_context.h"
>> +#include "pipe/p_state.h"
>>   #include "st_atom.h"
>>
>>
>> @@ -42,13 +43,19 @@ update_tess(struct st_context *st)
>>   {
>>      const struct gl_context *ctx = st->ctx;
>>      struct pipe_context *pipe = st->pipe;
>> +   struct pipe_tess_state state;
>>
>>      if (!pipe->set_tess_state)
>>         return;
>>
>> -   pipe->set_tess_state(pipe,
>> -                        ctx->TessCtrlProgram.patch_default_outer_level,
>> -                        ctx->TessCtrlProgram.patch_default_inner_level);
>> +   memcpy(state.default_outer_level,
>> +          ctx->TessCtrlProgram.patch_default_outer_level,
>> +          sizeof(state.default_outer_level));
>> +   memcpy(state.default_inner_level,
>> +          ctx->TessCtrlProgram.patch_default_inner_level,
>> +          sizeof(state.default_inner_level));
>> +
>> +   pipe->set_tess_state(pipe, &state);
>>   }
>>
>>
>> --
>> 2.5.5
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


More information about the mesa-dev mailing list