On Tue, Jun 14, 2016 at 12:30 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> 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.

I'd have to think about how it would work to deal w/ variants that
have params not wrapped in a struct.  It at least sounds annoying, and
I tended to think the benefits of using a struct where enough of a
justification to change this.  (Plus there are not many usages of this
API yet, so seemed like the perfect time to cleanup.)

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

*maybe*, but I can't think of anything..  right now it is only the
sampler_view and stream_output_target state that I handle "manually"..
but those are also kind of different from the rest since they are
already refcnt'd.  And I figured it was easier to just deal w/ those
manually than implement a 3rd type of state (CSO vs Param) in
rsq_state.py..

BR,
-R

> Nicolai
>
>>
>>    -ilia
>>
>> On Tue, Jun 14, 2016 at 11:57 AM, Rob Clark <robdcl...@gmail.com> wrote:
>>>
>>> From: Rob Clark <robcl...@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 <robcl...@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to