[waffle] [PATCH 05/12] waffle: add waffle_display_info_json()
Emil Velikov
emil.l.velikov at gmail.com
Sun Apr 24 10:42:24 UTC 2016
On 21 April 2016 at 21:25, Frank Henigman <fjhenigman at google.com> wrote:
> On Fri, Jan 8, 2016 at 7:40 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 6 January 2016 at 21:56, Frank Henigman <fjhenigman at google.com> wrote:
>>> Duplicate wflinfo functionality in the API, with the difference that the
>>> information is returned in JSON form.
>>> The function has a parameter for including platform-specific information,
>>> but it is ignored for now.
>>>
>>> Signed-off-by: Frank Henigman <fjhenigman at google.com>
>>> ---
>>> include/waffle/waffle.h | 5 +
>>> man/waffle_display.3.xml | 19 +++
>>> src/waffle/api/waffle_display.c | 284 +++++++++++++++++++++++++++++++++++++++-
>>> src/waffle/waffle.def.in | 1 +
>>> 4 files changed, 308 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/waffle/waffle.h b/include/waffle/waffle.h
>>> index df0218e..1800399 100644
>>> --- a/include/waffle/waffle.h
>>> +++ b/include/waffle/waffle.h
>>> @@ -214,6 +214,11 @@ bool
>>> waffle_display_supports_context_api(struct waffle_display *self,
>>> int32_t context_api);
>>>
>>> +#if WAFFLE_API_VERSION >= 0x0106
>>> +char*
>>> +waffle_display_info_json(struct waffle_display *self, bool platform_too);
>> The function does not work solely with the display, but it requires a
>> (bound) context. Thus it does not really fit waffle naming scheme. I'm
>> afraid that I'm short of suggestions though (barring my "returning
>> json formatted data sounds iffy, lets use tokens" rant from earlier)
>>
>>
>>> +#endif
>>> +
>>> union waffle_native_display*
>>> waffle_display_get_native(struct waffle_display *self);
>>>
>>> diff --git a/man/waffle_display.3.xml b/man/waffle_display.3.xml
>>> index 9896247..5358472 100644
>>> --- a/man/waffle_display.3.xml
>>> +++ b/man/waffle_display.3.xml
>>> @@ -24,6 +24,7 @@
>>> <refname>waffle_display</refname>
>>> <refname>waffle_display_connect</refname>
>>> <refname>waffle_display_disconnect</refname>
>>> + <refname>waffle_display_info_json</refname>
>>> <refname>waffle_display_supports_context_api</refname>
>>> <refname>waffle_display_get_native</refname>
>>> <refpurpose>class <classname>waffle_display</classname></refpurpose>
>>> @@ -58,6 +59,12 @@ struct waffle_display;
>>> </funcprototype>
>>>
>>> <funcprototype>
>>> + <funcdef>char* <function>waffle_display_info_json</function></funcdef>
>>> + <paramdef>struct waffle_display *<parameter>self</parameter></paramdef>
>>> + <paramdef>bool <parameter>platform_info</parameter></paramdef>
>>> + </funcprototype>
>>> +
>>> + <funcprototype>
>>> <funcdef>bool <function>waffle_display_supports_context_api</function></funcdef>
>>> <paramdef>struct waffle_display *<parameter>self</parameter></paramdef>
>>> <paramdef>int32_t <parameter>context_api</parameter></paramdef>
>>> @@ -129,6 +136,18 @@ struct waffle_display;
>>> </varlistentry>
>>>
>>> <varlistentry>
>>> + <term><function>waffle_display_info_json()</function></term>
>>> + <listitem>
>>> + <para>
>>> + Return a JSON string containing information about the current context on the given display, including Waffle platform and API, GL version/vendor/renderer and extensions.
>>> + If <parameter>platform_info</parameter> is true, platform-specific information (such as GLX or EGL versions and extensions) will be included as available.
>>> + Returns <constant>NULL</constant> on error.
>>> + The string should be deallocated with <citerefentry><refentrytitle><function>free</function></refentrytitle><manvolnum>3</manvolnum></citerefentry>.
>>> + </para>
>>> + </listitem>
>>> + </varlistentry>
>>> +
>>> + <varlistentry>
>>> <term><function>waffle_display_supports_context_api()</function></term>
>>> <listitem>
>>> <para>
>>> diff --git a/src/waffle/api/waffle_display.c b/src/waffle/api/waffle_display.c
>>> index fa19462..7abe2ef 100644
>>> --- a/src/waffle/api/waffle_display.c
>>> +++ b/src/waffle/api/waffle_display.c
>>> @@ -23,13 +23,61 @@
>>> // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>>> // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>>
>>> +#include <ctype.h>
>>> +#include <stdio.h>
>>> +
>>> #include "api_priv.h"
>>>
>>> -#include "wcore_error.h"
>>> +#include "json.h"
>>> +
>>> +#include "wcore_context.h"
>>> #include "wcore_display.h"
>>> +#include "wcore_error.h"
>>> #include "wcore_platform.h"
>>> #include "wcore_util.h"
>>>
>>> +typedef unsigned int GLint;
>>> +typedef unsigned int GLenum;
>>> +typedef unsigned char GLubyte;
>>> +
>>> +enum {
>>> + // Copied from <GL/gl*.h>.
>>> + GL_NO_ERROR = 0,
>>> +
>>> + GL_CONTEXT_FLAGS = 0x821e,
>>> + GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT = 0x00000001,
>>> + GL_CONTEXT_FLAG_DEBUG_BIT = 0x00000002,
>>> + GL_CONTEXT_FLAG_ROBUST_ACCESS_BIT_ARB = 0x00000004,
>>> +
>>> + GL_VENDOR = 0x1F00,
>>> + GL_RENDERER = 0x1F01,
>>> + GL_VERSION = 0x1F02,
>>> + GL_EXTENSIONS = 0x1F03,
>>> + GL_NUM_EXTENSIONS = 0x821D,
>>> + GL_SHADING_LANGUAGE_VERSION = 0x8B8C,
>>> +};
>>> +
>>> +#ifndef _WIN32
>>> +#define APIENTRY
>>> +#else
>>> +#ifndef APIENTRY
>>> +#define APIENTRY __stdcall
>>> +#endif
>>> +#endif
>>> +
>>> +static GLenum (APIENTRY *glGetError)(void);
>>> +static void (APIENTRY *glGetIntegerv)(GLenum pname, GLint *params);
>>> +static const GLubyte * (APIENTRY *glGetString)(GLenum name);
>>> +static const GLubyte * (APIENTRY *glGetStringi)(GLenum name, GLint i);
>>> +
>>> +#if defined(__GNUC__)
>>> +#define NORETURN __attribute__((noreturn))
>>> +#elif defined(_MSC_VER)
>>> +#define NORETURN __declspec(noreturn)
>>> +#else
>>> +#define NORETURN
>>> +#endif
>>> +
>>> WAFFLE_API struct waffle_display*
>>> waffle_display_connect(const char *name)
>>> {
>>> @@ -90,6 +138,240 @@ waffle_display_supports_context_api(
>>> context_api);
>>> }
>>>
>>> +static int
>>> +parse_version(const char *version)
>>> +{
>>> + int count, major, minor;
>>> +
>>> + if (version == NULL)
>>> + return 0;
>>> +
>>> + while (*version != '\0' && !isdigit(*version))
>>> + version++;
>>> +
>>> + count = sscanf(version, "%d.%d", &major, &minor);
>>> + if (count != 2)
>>> + return 0;
>>> +
>>> + if (minor > 9)
>>> + return 0;
>>> +
>>> + return (major * 10) + minor;
>>> +}
>>> +
>>> +static void
>>> +add_context_flags(struct json *jj)
>>> +{
>>> + static struct {
>>> + GLint flag;
>>> + char *str;
>>> + } flags[] = {
>>> + { GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT, "FORWARD_COMPATIBLE" },
>>> + { GL_CONTEXT_FLAG_DEBUG_BIT, "DEBUG" },
>>> + { GL_CONTEXT_FLAG_ROBUST_ACCESS_BIT_ARB, "ROBUST_ACCESS" },
>>> + };
>>> + int flag_count = sizeof(flags) / sizeof(flags[0]);
>>> + GLint context_flags = 0;
>>> +
>>> + glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags);
>>> + if (glGetError() != GL_NO_ERROR)
>>> + return json_append(jj, json_str("WFLINFO_GL_ERROR"));
>>> +
>>> + if (context_flags == 0)
>>> + return json_append(jj, json_num(0));
>>> +
>>> + for (int i = 0; i < flag_count; i++) {
>>> + if ((flags[i].flag & context_flags) != 0) {
>>> + json_append(jj, json_str(flags[i].str));
>>> + context_flags = context_flags & ~flags[i].flag;
>>> + }
>>> + }
>>> + for (int i = 0; context_flags != 0; context_flags >>= 1, i++) {
>>> + if ((context_flags & 1) != 0) {
>>> + json_append(jj, json_num(1 << i));
>>> + }
>>> + }
>>> +}
>>> +
>>> +static void
>>> +add_extensions(struct json *jj, bool use_stringi)
>>> +{
>>> + GLint count = 0, i;
>>> + const char *ext;
>>> +
>>> + if (use_stringi) {
>>> + glGetIntegerv(GL_NUM_EXTENSIONS, &count);
>>> + if (glGetError() != GL_NO_ERROR) {
>>> + json_append(jj, json_str("WFLINFO_GL_ERROR"));
>>> + } else {
>>> + for (i = 0; i < count; i++) {
>>> + ext = (const char *) glGetStringi(GL_EXTENSIONS, i);
>>> + if (glGetError() != GL_NO_ERROR)
>>> + ext = "WFLINFO_GL_ERROR";
>>> + json_append(jj, json_str(ext));
>>> + }
>>> + }
>>> + } else {
>>> + const char *extensions = (const char *) glGetString(GL_EXTENSIONS);
>>> + if (glGetError() != GL_NO_ERROR)
>>> + json_append(jj, json_str("WFLINFO_GL_ERROR"));
>>> + else
>>> + json_append(jj, json_split(extensions, " "));
>>> + }
>>> +}
>>> +
>>> +static void
>>> +add_generic_info(struct json *jj, struct wcore_context *ctx)
>>> +{
>>> + int32_t dl;
>>> + //XXX this pattern seems to occur repeatedly - do we need two sets of enums?
>> I'm afraid we do. The whole topic is a bit messy, but the gist is that
>> - because we have the GL* library that does not imply that we can have
>> a context of said API and vice-versa.
>>
>>> + switch (ctx->context_api) {
>>> + case WAFFLE_CONTEXT_OPENGL: dl = WAFFLE_DL_OPENGL; break;
>>> + case WAFFLE_CONTEXT_OPENGL_ES1: dl = WAFFLE_DL_OPENGL_ES1; break;
>>> + case WAFFLE_CONTEXT_OPENGL_ES2: dl = WAFFLE_DL_OPENGL_ES2; break;
>>> + case WAFFLE_CONTEXT_OPENGL_ES3: dl = WAFFLE_DL_OPENGL_ES3; break;
>>> + default:
>>> + abort();
>> This feels excessive. Set an error and bail out ?
>
> I'll change to assert(false). I think we want at least that, because
> the value has been validated by this point so we should never get
> here.
>
Good point. I've recently went through waffle and unified things to
follow this approach - use assert if we've already validated things.
Patches are on the mailing list - "... replace wcore_error_internal
with assert" - feel free to take a look.
>>
>>> + break;
>>> + }
>>> +
>>> + glGetError = waffle_dl_sym(dl, "glGetError");
>>> + if (!glGetError)
>>> + return json_append(jj, NULL);
>>> +
>>> + glGetIntegerv = waffle_dl_sym(dl, "glGetIntegerv");
>>> + if (!glGetIntegerv)
>>> + return json_append(jj, NULL);
>>> +
>>> + glGetString = waffle_dl_sym(dl, "glGetString");
>>> + if (!glGetString)
>>> + return json_append(jj, NULL);
>>> +
>>> + // Retrieving GL functions is tricky. When glGetStringi is supported, here
>>> + // are some boggling variations as of 2014-11-19:
>>> + // - Mali drivers on EGL 1.4 expose glGetStringi statically from
>>> + // libGLESv2 but not dynamically from eglGetProcAddress. The EGL 1.4 spec
>>> + // permits this behavior.
>>> + // - EGL 1.5 requires that eglGetStringi be exposed dynamically through
>>> + // eglGetProcAddress. Exposing statically with dlsym is optional.
>>> + // - Windows requires that glGetStringi be exposed dynamically from
>>> + // wglGetProcAddress. Exposing statically from GetProcAddress (Window's
>>> + // dlsym equivalent) is optional.
>>> + // - Mesa drivers expose glGetStringi statically from libGL and libGLESv2
>>> + // and dynamically from eglGetProcAddress and glxGetProcAddress.
>>> + // - Mac exposes glGetStringi only statically.
>>> + //
>>> + // Try waffle_dl_sym before waffle_get_proc_address because
>>> + // (1) egl/glXProcAddress can return invalid non-null pointers for
>>> + // unsupported functions and (2) dlsym returns non-null if and only if the
>>> + // library exposes the symbol.
>>> + glGetStringi = waffle_dl_sym(dl, "glGetStringi");
>>> + if (!glGetStringi) {
>>> + glGetStringi = waffle_get_proc_address("glGetStringi");
>>> + }
>>> +
>>> + while(glGetError() != GL_NO_ERROR) {
>>> + /* Clear all errors */
>>> + }
>>> +
>> As mentioned elsewhere - why the loop (and yes same question goes for
>> the original in wflinfo) ?
>
> According to the man page "glGetError should always be called in a
> loop, until it returns GL_NO_ERROR, if all error flags are to be
> reset."
>
Indeed it does. I wonder if any drivers implement error. queuqueuet
sure if this is the right terminology here), which the spec implies
with the text.
Related: most of piglit (something like 90+%) does not use a loop
similar to all of xserver. Haven't looked at dEQP although I'm leaning
that it might be in a similar boat.
>>
>>> + const char *vendor = (const char *) glGetString(GL_VENDOR);
>>> + if (glGetError() != GL_NO_ERROR || vendor == NULL) {
>>> + vendor = "WFLINFO_GL_ERROR";
>>> + }
>>> +
>>> + const char *renderer = (const char *) glGetString(GL_RENDERER);
>>> + if (glGetError() != GL_NO_ERROR || renderer == NULL) {
>>> + renderer = "WFLINFO_GL_ERROR";
>>> + }
>>> +
>>> + const char *version_str = (const char *) glGetString(GL_VERSION);
>>> + if (glGetError() != GL_NO_ERROR || version_str == NULL) {
>>> + version_str = "WFLINFO_GL_ERROR";
>>> + }
>>> +
>> Please drop the extra curly brackets from the last 4 if statements.
>> Afaict with MSVC2013 U4 (our min requirement) things should just work
>> ?
>>
>>> + assert(ctx->display->platform->waffle_platform);
>>> + const char *platform =
>>> + wcore_enum_to_string(ctx->display->platform->waffle_platform);
>>> + assert(platform != NULL);
>>> +
>>> + assert(ctx->context_api);
>>> + const char *api = wcore_enum_to_string(ctx->context_api);
>>> + assert(api != NULL);
>>> +
>>> + json_appendv(jj,
>>> + "waffle", "{",
>>> + "platform", json_str(platform),
>>> + "api", json_str(api),
>>> + "}",
>>> + "opengl", "{",
>>> + "vendor", json_str(vendor),
>>> + "renderer", json_str(renderer),
>>> + "version", json_str(version_str),
>>> + "}", "");
>>> +
>>> + int version = parse_version(version_str);
>>> +
>>> + if (ctx->context_api == WAFFLE_CONTEXT_OPENGL && version >= 31) {
>>> + json_appendv(jj, "context_flags", "[", "");
>>> + add_context_flags(jj);
>>> + json_append(jj, "]");
>>> + }
>>> +
>>> + // OpenGL and OpenGL ES >= 3.0 support glGetStringi(GL_EXTENSION, i).
>>> + const bool use_getstringi = version >= 30;
>>> +
>>> + if (!glGetStringi && use_getstringi)
>>> + return json_append(jj, NULL);
>>> +
>>> + // There are two exceptional cases where wflinfo may not get a
>>> + // version (or a valid version): one is in gles1 and the other
>>> + // is GL < 2.0. In these cases do not return WFLINFO_GL_ERROR,
>>> + // return None. This is preferable to returning WFLINFO_GL_ERROR
>>> + // because it creates a consistant interface for parsers
>>> + const char *language_str = "None";
>>> + if ((ctx->context_api == WAFFLE_CONTEXT_OPENGL && version >= 20) ||
>>> + ctx->context_api == WAFFLE_CONTEXT_OPENGL_ES2 ||
>>> + ctx->context_api == WAFFLE_CONTEXT_OPENGL_ES3) {
>>> + language_str = (const char *) glGetString(GL_SHADING_LANGUAGE_VERSION);
>>> + if (glGetError() != GL_NO_ERROR || language_str == NULL) {
>>> + language_str = "WFLINFO_GL_ERROR";
>>> + }
>>> + }
>>> +
>>> + json_appendv(jj, "shading_language_version", json_str(language_str), "");
>>> + json_appendv(jj, "extensions", "[", "");
>>> + add_extensions(jj, use_getstringi);
>>> + json_append(jj, "]");
>>> +}
>>> +
>>> +WAFFLE_API char*
>> Please add space between char and *
>>
>>> +waffle_display_info_json(struct waffle_display *self, bool platform_too)
>>> +{
>>> + struct wcore_display *wc_self = wcore_display(self);
>>> +
>>> + const struct api_object *obj_list[] = {
>>> + wc_self ? &wc_self->api : NULL,
>>> + };
>>> +
>>> + if (!api_check_entry(obj_list, 1))
>>> + return NULL;
>>> +
>>> + if (!wc_self->current_context) {
>>> + wcore_errorf(WAFFLE_ERROR_UNKNOWN, "no current context");
>>> + return NULL;
>>> + }
>>> +
>>> + struct json *jj = json_init();
>>> + if (!jj)
>> Set the error state ?
>
> It will only fail if it can't get memory, in which case I don't see
> the point in setting error state.
> Throughout waffle we just return NULL if calloc() fails, without
> setting an error.
>
We consistently use waffle_[mc]alloc wrappers which themselves set the
error state. Although looking at the json implementation... that can
only happen in case of -ENOMEM, so if we go with my suggestion (in the
json patch) and add a note here that'll be amazing.
If you can think of anything better than "Any of the following json
functions can only fail due to ENOMEM, and waffle_[mc]alloc already
sets the error state", please go ahead.
>>
>>> + return NULL;
>>> +
>>> + json_appendv(jj, "{", "generic", "{", "");
>>> + add_generic_info(jj, wc_self->current_context);
>>> + json_appendv(jj, "}", "}", "");
>>> +
>> A similar question if the json library fails at some point ?
>
> Similar answer. (:
>
With the above said, feeding the user some garbage/incomplete data
sounds very wrong imho. In a similar way you expect a function to not
put data in the output pointer if it fails, right ?
Thanks
Emil
More information about the waffle
mailing list