[Mesa-dev] [PATCH 3/6] gallium/u_blitter: split out a helper for common clear state

[Marek Olšák]

On Tue, Aug 16, 2016 at 4:02 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Mon, Aug 15, 2016 at 10:44 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Sat, Aug 13, 2016 at 8:57 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Sat, Aug 13, 2016 at 2:23 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>>> ---
>>>>  src/gallium/auxiliary/util/u_blitter.c | 38 ++++++++++++++++++++++------------
>>>>  src/gallium/auxiliary/util/u_blitter.h |  5 +++++
>>>>  2 files changed, 30 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c
>>>> index 9fbef9b..e008100 100644
>>>> --- a/src/gallium/auxiliary/util/u_blitter.c
>>>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>>>> @@ -1268,19 +1268,13 @@ static void *get_clear_blend_state(struct blitter_context_priv *ctx,
>>>>     return ctx->blend_clear[index];
>>>>  }
>>>>
>>>> -static void util_blitter_clear_custom(struct blitter_context *blitter,
>>>> -                                      unsigned width, unsigned height,
>>>> -                                      unsigned num_layers,
>>>> -                                      unsigned clear_buffers,
>>>> -                                      const union pipe_color_union *color,
>>>> -                                      double depth, unsigned stencil,
>>>> -                                      void *custom_blend, void *custom_dsa)
>>>> +void util_blitter_common_clear_setup(struct blitter_context *blitter,
>>>> +                                     unsigned width, unsigned height,
>>>> +                                     unsigned clear_buffers,
>>>> +                                     void *custom_blend, void *custom_dsa)
>>>>  {
>>>>     struct blitter_context_priv *ctx = (struct blitter_context_priv*)blitter;
>>>>     struct pipe_context *pipe = ctx->base.pipe;
>>>> -   struct pipe_stencil_ref sr = { { 0 } };
>>>> -
>>>> -   assert(ctx->has_layered || num_layers <= 1);
>>>>
>>>>     util_blitter_set_running_flag(blitter);
>>>>     blitter_check_saved_vertex_states(ctx);
>>>
>>> so actually, I need to squash in:
>>>
>>> ----------
>>> diff --git a/src/gallium/auxiliary/util/u_blitter.c
>>> b/src/gallium/auxiliary/util/u_blitter.c
>>> index e008100..c0d3808 100644
>>> --- a/src/gallium/auxiliary/util/u_blitter.c
>>> +++ b/src/gallium/auxiliary/util/u_blitter.c
>>> @@ -1281,6 +1281,12 @@ void util_blitter_common_clear_setup(struct
>>> blitter_context *blitter,
>>>     blitter_check_saved_fragment_states(ctx);
>>>     blitter_disable_render_cond(ctx);
>>>
>>> +   /* this will end up getting set again in
>>> blitter_set_common_draw_rect_state()
>>> +    * but needs to be here for drivers using util_blitter_common_clear_setup()
>>> +    * but not util_blitter_clear_custom()
>>> +    */
>>> +   pipe->bind_rasterizer_state(pipe, ctx->rs_state);
>>> +
>>>     /* bind states */
>>>     if (custom_blend) {
>>>        pipe->bind_blend_state(pipe, custom_blend);
>>> ----------
>>>
>>> I don't think there should be any harm in doing
>>> pipe->bind_rasterizer_state() twice for drivers using
>>> util_blitter_clear_custom(), and this is easier that adding a new
>>> helper (since ctx->rs_state is not visible outside of u_blitter.c),
>>> but if someone sees a problem with binding rasterizer state twice then
>>> I could make it a new helper fxn instead.
>>
>> For clarity, it would be better to expose ctx->rs_state to drivers
>> (e.g. via a function) instead of this hack.
>
> since it only amounts to a few lines of code, I think I'll just create
> my own rs_state in fd_context, and go back to the original patch here
> without anything squashed in.  Was that your only objection to patch
> 3?

Yes, the original patch is:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek