[Mesa-dev] [PATCH 5/6] st/mesa: set correct PIPE_CLEAR_COLORn flags

[Marek Olšák]

We can easily change the functions, so that they take pipe_box instead
of pipe_surface. It's not a problem. The state tracker or gallium/util
will have to provide a fallback path for compressed and
non-renderable formats anyway.

Marek

On Fri, Dec 6, 2013 at 2:29 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Oh I wasn't aware of that extension.
> It seems though those functions aren't quite ideally suited for that
> neither. First, they require surfaces, whereas the OpenGL extension is
> done specifically so the texture may be cleared even if the texture is
> non-renderable (hence surface creation may fail).
> Second, the functions do not allow to specify depth/zoffset currently as
> they always clear all attached layers.
> Those should be fixable though I guess.
>
> Roland
>
> Am 05.12.2013 23:02, schrieb Marek Olšák:
>> We'll probably use clear_render_target for GL_ARB_clear_texture (GL
>> 4.4), so it will be useful finally.
>>
>> Marek
>>
>> On Thu, Dec 5, 2013 at 10:49 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>> Am 05.12.2013 18:53, schrieb Marek Olšák:
>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>
>>>> This also fixes the clear_with_quad function for glClearBuffer.
>>>> ---
>>>>  src/mesa/state_tracker/st_cb_clear.c | 37 +++++++++++++++++++++++++-----------
>>>>  1 file changed, 26 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_cb_clear.c b/src/mesa/state_tracker/st_cb_clear.c
>>>> index 5a7a00c..887e58b 100644
>>>> --- a/src/mesa/state_tracker/st_cb_clear.c
>>>> +++ b/src/mesa/state_tracker/st_cb_clear.c
>>>> @@ -213,8 +213,7 @@ draw_quad(struct st_context *st,
>>>>   * ctx->DrawBuffer->_X/Ymin/max fields.
>>>>   */
>>>>  static void
>>>> -clear_with_quad(struct gl_context *ctx,
>>>> -                GLboolean color, GLboolean depth, GLboolean stencil)
>>>> +clear_with_quad(struct gl_context *ctx, unsigned clear_buffers)
>>>>  {
>>>>     struct st_context *st = st_context(ctx);
>>>>     const struct gl_framebuffer *fb = ctx->DrawBuffer;
>>>> @@ -253,7 +252,7 @@ clear_with_quad(struct gl_context *ctx,
>>>>     {
>>>>        struct pipe_blend_state blend;
>>>>        memset(&blend, 0, sizeof(blend));
>>>> -      if (color) {
>>>> +      if (clear_buffers & PIPE_CLEAR_COLOR) {
>>>>           int num_buffers = ctx->Extensions.EXT_draw_buffers2 ?
>>>>                             ctx->DrawBuffer->_NumColorDrawBuffers : 1;
>>>>           int i;
>>>> @@ -261,6 +260,9 @@ clear_with_quad(struct gl_context *ctx,
>>>>           blend.independent_blend_enable = num_buffers > 1;
>>>>
>>>>           for (i = 0; i < num_buffers; i++) {
>>>> +            if (!(clear_buffers & (PIPE_CLEAR_COLOR0 << i)))
>>>> +               continue;
>>>> +
>>>>              if (ctx->Color.ColorMask[i][0])
>>>>                 blend.rt[i].colormask |= PIPE_MASK_R;
>>>>              if (ctx->Color.ColorMask[i][1])
>>>> @@ -281,13 +283,13 @@ clear_with_quad(struct gl_context *ctx,
>>>>     {
>>>>        struct pipe_depth_stencil_alpha_state depth_stencil;
>>>>        memset(&depth_stencil, 0, sizeof(depth_stencil));
>>>> -      if (depth) {
>>>> +      if (clear_buffers & PIPE_CLEAR_DEPTH) {
>>>>           depth_stencil.depth.enabled = 1;
>>>>           depth_stencil.depth.writemask = 1;
>>>>           depth_stencil.depth.func = PIPE_FUNC_ALWAYS;
>>>>        }
>>>>
>>>> -      if (stencil) {
>>>> +      if (clear_buffers & PIPE_CLEAR_STENCIL) {
>>>>           struct pipe_stencil_ref stencil_ref;
>>>>           memset(&stencil_ref, 0, sizeof(stencil_ref));
>>>>           depth_stencil.stencil[0].enabled = 1;
>>>> @@ -371,6 +373,19 @@ is_scissor_enabled(struct gl_context *ctx, struct gl_renderbuffer *rb)
>>>>
>>>>
>>>>  /**
>>>> + * Return if all of the color channels are masked.
>>>> + */
>>>> +static INLINE GLboolean
>>>> +is_color_disabled(struct gl_context *ctx, int i)
>>>> +{
>>>> +   return !ctx->Color.ColorMask[i][0] &&
>>>> +          !ctx->Color.ColorMask[i][1] &&
>>>> +          !ctx->Color.ColorMask[i][2] &&
>>>> +          !ctx->Color.ColorMask[i][3];
>>>> +}
>>> Technically you could also return true if not all channels are masked,
>>> as long as these channels don't exist in the rb. I guess though that gets
>>> a bit too complicated...
>>>
>>>
>>>> +
>>>> +/**
>>>>   * Return if any of the color channels are masked.
>>>>   */
>>>>  static INLINE GLboolean
>>>> @@ -427,11 +442,14 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>>>>              if (!strb || !strb->surface)
>>>>                 continue;
>>>>
>>>> +            if (is_color_disabled(ctx, colormask_index))
>>>> +               continue;
>>>> +
>>>>              if (is_scissor_enabled(ctx, rb) ||
>>>>                  is_color_masked(ctx, colormask_index))
>>>> -               quad_buffers |= PIPE_CLEAR_COLOR;
>>>> +               quad_buffers |= PIPE_CLEAR_COLOR0 << i;
>>>>              else
>>>> -               clear_buffers |= PIPE_CLEAR_COLOR;
>>>> +               clear_buffers |= PIPE_CLEAR_COLOR0 << i;
>>>>           }
>>>>        }
>>>>     }
>>>> @@ -464,10 +482,7 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
>>>>      */
>>>>     if (quad_buffers) {
>>>>        quad_buffers |= clear_buffers;
>>>> -      clear_with_quad(ctx,
>>>> -                      quad_buffers & PIPE_CLEAR_COLOR,
>>>> -                      quad_buffers & PIPE_CLEAR_DEPTH,
>>>> -                      quad_buffers & PIPE_CLEAR_STENCIL);
>>>> +      clear_with_quad(ctx, quad_buffers);
>>>>     } else if (clear_buffers) {
>>>>        /* We can't translate the clear color to the colorbuffer format,
>>>>         * because different colorbuffers may have different formats.
>>>>
>>>
>>> Series makes sense to me if you don't want to use the
>>> clear_render_target() function for that instead. And I guess the latter
>>> hasn't many supporters...
>>>
>>> Roland