[Mesa-dev] [PATCH 9/9] mesa/st: take ownership rather than adding reference for new renderbuffers

[Emil Velikov]

On 8 April 2017 at 02:04, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> This avoids locking in the reference calls and fixes a leak after the
> RefCount initialisation was change from 0 to 1.
>
> Fixes: 32141e53d1520 (mesa: tidy up renderbuffer RefCount initialisation)
> ---
>  src/mesa/state_tracker/st_manager.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
> index f469ebd..48daab6 100644
> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -305,27 +305,38 @@ st_framebuffer_add_renderbuffer(struct st_framebuffer *stfb,
>     }
>
>     if (format == PIPE_FORMAT_NONE)
>        return FALSE;
>
>     rb = st_new_renderbuffer_fb(format, stfb->iface->visual->samples, sw);
>     if (!rb)
>        return FALSE;
>
>     if (idx != BUFFER_DEPTH) {
> -      _mesa_add_renderbuffer(&stfb->Base, idx, rb);
> +      _mesa_add_renderbuffer_without_ref(&stfb->Base, idx, rb);
>     }
>     else {
> -      if (util_format_get_component_bits(format, UTIL_FORMAT_COLORSPACE_ZS, 0))
> -         _mesa_add_renderbuffer(&stfb->Base, BUFFER_DEPTH, rb);
> -      if (util_format_get_component_bits(format, UTIL_FORMAT_COLORSPACE_ZS, 1))
> -         _mesa_add_renderbuffer(&stfb->Base, BUFFER_STENCIL, rb);
> +      bool rb_ownership_taken = false;
> +      if (util_format_get_component_bits(format,
> +                                         UTIL_FORMAT_COLORSPACE_ZS, 0)) {
> +         _mesa_add_renderbuffer_without_ref(&stfb->Base, BUFFER_DEPTH, rb);
> +         rb_ownership_taken = true;
> +      }
> +      if (util_format_get_component_bits(format,
> +                                         UTIL_FORMAT_COLORSPACE_ZS, 1)) {
> +         if (rb_ownership_taken)
> +            _mesa_add_renderbuffer(&stfb->Base, BUFFER_STENCIL, rb);
> +         else {
> +            _mesa_add_renderbuffer_without_ref(&stfb->Base, BUFFER_STENCIL,
> +                                               rb);
> +         }
> +      }
>     }
>
Nitpick: return early for idx != BUFFER_DEPTH and re-indent the rest
to make it easier on the eyes.

   if (idx != BUFFER_DEPTH) {
      _mesa_add_renderbuffer_without_ref(&stfb->Base, idx, rb);
      return TRUE;
   }

   bool rb_ownership_taken = false;
   if (util_format_get_component_bits(format, UTIL_FORMAT_COLORSPACE_ZS, 0)) {
      _mesa_add_renderbuffer_without_ref(&stfb->Base, BUFFER_DEPTH, rb);
      rb_ownership_taken = true;
   }

   if (util_format_get_component_bits(format, UTIL_FORMAT_COLORSPACE_ZS, 1)) {
      if (rb_ownership_taken)
         _mesa_add_renderbuffer(&stfb->Base, BUFFER_STENCIL, rb);
      else
         _mesa_add_renderbuffer_without_ref(&stfb->Base, BUFFER_STENCIL, rb);
   }

For the series (ideally with the nitpick, but meh)
Reviewed-by; Emil Velikov <emil.velikov at collabora.com>

-Emil