[Mesa-dev] [PATCH] gallium: create a debug screen wrapper around swrast screens

[Emil Velikov]

On 20 November 2016 at 16:28, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> Thanks Marek.
>
> Emil, I'd appreciate it if you could have a look as well. I believe
> you were the last to touch these helpers. I'm highly confused by the
> flow of things, and mainly want to ensure that we don't warp the same
> screen *twice*.
>
Thanks for the poke.

>>> diff --git a/src/gallium/auxiliary/target-helpers/inline_sw_helper.h b/src/gallium/auxiliary/target-helpers/inline_sw_helper.h
>>> index 5bb77a5..3d9d824 100644
>>> --- a/src/gallium/auxiliary/target-helpers/inline_sw_helper.h
>>> +++ b/src/gallium/auxiliary/target-helpers/inline_sw_helper.h
>>> @@ -55,6 +55,9 @@ sw_screen_create_named(struct sw_winsys *winsys, const char *driver)
>>>        screen = swr_create_screen(winsys);
>>>  #endif
>>>
>>> +   if (screen)
>>> +      screen = debug_screen_wrap(screen);
>>> +
Hmm things are pretty nice in here. We cannot have this hunk since
every user (old school static only modules and pipe-foo) explicitly
wraps the screen.

>>>     return screen;
>>>  }
>>>
>>> diff --git a/src/gallium/auxiliary/target-helpers/sw_helper.h b/src/gallium/auxiliary/target-helpers/sw_helper.h
>>> index 5e4e9f7..d1945ad 100644
>>> --- a/src/gallium/auxiliary/target-helpers/sw_helper.h
>>> +++ b/src/gallium/auxiliary/target-helpers/sw_helper.h
>>> @@ -57,6 +57,9 @@ sw_screen_create_named(struct sw_winsys *winsys, const char *driver)
>>>        screen = swr_create_screen(winsys);
>>>  #endif
>>>
>>> +   if (screen)
>>> +      screen = debug_screen_wrap(screen);
>>> +
Option A: is to keep only the sw_helper.h change.

Option B:
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
@@ -301,9 +301,16 @@ pipe_loader_sw_create_screen(struct
pipe_loader_device *dev)

   screen = sdev->dd->create_screen(sdev->ws);
   if (!screen)
-      sdev->ws->destroy(sdev->ws);
+      goto fail;

+#if GALLIUM_STATIC_TARGETS
+   screen = debug_screen_wrap(screen);
+#endif
   return screen;
+
+fail:
+   sdev->ws->destroy(sdev->ws);
+   return NULL;
}

If you like A, please add a comment - feel free to reuse mine. I'm
fine with either solution, pre-emptively
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil
P.S. In general the wrapper drivers are pretty inconsistent (nasty?) -
some error out, others continue, iirc some could even crash in OOM
conditions.