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