[waffle] [PATCH 03/12] core: store current context in wcore_display

[Chad Versace]

n 01/08/2016 04:15 AM, Emil Velikov wrote:
> On 6 January 2016 at 21:56, Frank Henigman <fjhenigman at google.com> wrote:
>> For core functions that need to know the current context, like the
>> forthcoming wflinfo-like function.
>>
>> Signed-off-by: Frank Henigman <fjhenigman at google.com>
>> ---
>>  src/waffle/api/waffle_gl_misc.c | 11 +++++++----
>>  src/waffle/core/wcore_display.c |  1 +
>>  src/waffle/core/wcore_display.h |  2 ++
>>  3 files changed, 10 insertions(+), 4 deletions(-)

>> diff --git a/src/waffle/core/wcore_display.c b/src/waffle/core/wcore_display.c
>> index 18262c3..bcaeacb 100644
>> --- a/src/waffle/core/wcore_display.c
>> +++ b/src/waffle/core/wcore_display.c
>> @@ -52,6 +52,7 @@ wcore_display_init(struct wcore_display *self,
>>      mtx_unlock(&mutex);
>>
>>      self->platform = platform;
>> +    self->current_context = NULL;

> We calloc the struct so we should need this ?

I think the explicit assignment to NULL is appropriate here, as the call to calloc
is in another file. The explicit assignment makes it easy to *locally* verify that
the code is correct. If the call to calloc were local to this function, then I would feel
differently.


>> diff --git a/src/waffle/core/wcore_display.h b/src/waffle/core/wcore_display.h
>> index 6e374e3..1ccff6f 100644

>>  struct wcore_display {
>>      struct api_object api;
>> +    struct wcore_context *current_context;

> Not 100% if wcore_display is the "right" struct, but keeping a
> reference in waffle sounds great imho. The alternative (using every
> permutation of GetCurrentContext) does feel like an overkill.

Waffle should keep a reference to the current context, but storing it in the
display is incorrect. The problem is that, at least in GLX and EGL, there
exists no context current to the *display*. Instead, a context may be current
to a *thread*. The distinction is significant, because a single display may be
bound to multiple threads, with a different context bound in each thread. In
other words, a display may have multiple current contexts, one per thread.

As a consequence, the thread's current context must be stored in thread-local
storage.

In response to this patch, I wrote my own patch [1] on my personal branch
'get-current' [2] that does exactly that: adds the current context and display
to Waffle's TLS. I plan on committing the patch after I submit to Intel's CI
and verify it doesn't interfere with any Piglit tests. (Intel's Mesa CI is down
for maintenance for today).

[1] https://github.com/chadversary/waffle/commit/1fdf3bd7f68bd1efa703c2cec4cd8a193c2182b5
[2] https://github.com/chadversary/waffle/commits/get-current

For reference, I quote below a comment from the patch that explains in more
detail the topic of "current objects".

Frank, wherever your patch series uses wcore_display::current_context, it should use
wcore_tinfo::current_context instead.

---- snip ----

/// A note on the current display and context:
///
///   EGL allows a thread to have current one the below combinations of display,
///   context, and surface.
///
///     a. No display, no context, no surface.
///
///        This case occurs when the thread has not previously called
///        eglMakeCurrent(), or when the thread has calls
///        eglMakeCurrent(EGL_DISPLAY_NONE, EGL_NO_SURFACE, EGL_NO_SURFACE,
///        EGL_NO_CONTEXT). Note that the EGL 1.5 specification mandates that
///        calling eglMakeCurrent() with EGL_DISPLAY_NONE generate an error,
///        but some implementations allow it anyway according to the footnote
///        in the EGL 1.5 specification (2014.08.27), p59:
///
///          Some implementations have chosen to allow EGL_NO_DISPLAY as
///          a valid dpy parameter for eglMakeCurrent. This behavior is not
///          portable to all EGL implementations, and should be considered as
///          an undocumented vendor extension.
///
///     b. One display, no context, no surface.
///
///        This case occurs when the thread calls eglMakeCurrent() with
///        a valid display but no context.
///
///     c. One display, one context, no surface.
///
///        Supported by EGL 1.5, EGL_KHR_surfaceless_context, and
///        EGL_KHR_create_context.
///
///     d. One display; one context, and both a draw and read surface.
///
///        The classic case.
///
///    EGL permits a context to be current to at most one thread. Same for
///    surfaces.  However, a display may be current to multiple threads.
///
///    Therefore, to support waffle_get_current(), Waffle must track all three
///    objects (display, context, and waffle_window) in thread-specific storage.
///    It is insufficient to maintain a reference to the current context and
///    window in the current display, as there may exist multiple contexts and
///    surfaces, each current in different threads, but all children to the same
///    display.
///
struct wcore_tinfo {
    ...
    struct wcore_display *current_display;
    struct wcore_window *current_window;
    struct wcore_context *current_context;
    ...
};