[waffle] [PATCH] waffle utils: add wflinfo utility

Chad Versace chad.versace at linux.intel.com
Fri Dec 20 14:31:15 PST 2013


On 12/19/2013 11:31 AM, Jordan Justen wrote:
> glxinfo for waffle, but using the command line interface
> of waffle's gl_basic test.

This is great. I've wanted to write a windowsystem-indepent utility
like this for a long time. Thanks for submitting it.

Comments below.

> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>   Tested on Intel Ivy Bridge with Mesa.
>
>   Seemed to work well with platforms glx, gbm and x11_egl,
>   and with apis gl, gles1, gles2 and gles3.

I tested with Wayland, and it works there too.

I prefer to commit code for "exotic" platforms like Android and Mac
only after the code is tested. I dislike committing untested (and
hence likely broken) placeholder code.

I'll test the utility on Mac before committing. For Android, though,
please submit the Android.mk as a separate patch that I'll merge
after an Android dev tests it.

>   One question: Should we try to determine a default for
>   platform/api and make those parameters optional?

No. If we select a mechanism for choosing a default platform, then
that mechanism must be preserved indefinitey for backwards compatibility.
That default mechanism, however, will likely become inappropriate for
distros that transition to Wayland.

I prefer that we do not attempt to predict the future, and as a consequence
do not encode a X11/Wayland preference.

Once you throw GBM into the mix too, the mechanism for choosing a default platform
looks very messy to me.

For similar reasons, I prefer to not choose a default GL api.

If any default behavior makes sense at all, I think it is to attempt to
exhaustively print info on all apis and profiles supported by the platform.
However, let's not hastily try to implement that. Let's get the wflinfo
merged as-is with no default behavior, and discuss later if any default behavior
makes sense.

>   Another question: I copied this from gl_basic.c, which
>   actually attempts to render. Am I perhaps doing more than
>   required for creating the context?

Yes, you're doing a little bit extra. Comments below.

>   src/CMakeLists.txt       |   1 +
>   src/utils/Android.mk     |  20 ++
>   src/utils/CMakeLists.txt |  16 ++
>   src/utils/wflinfo.c      | 642 +++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 679 insertions(+)
>   create mode 100644 src/utils/Android.mk
>   create mode 100644 src/utils/CMakeLists.txt
>   create mode 100644 src/utils/wflinfo.c
>
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 661bdd7..aa6d3c3 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -2,4 +2,5 @@ if(waffle_build_tests)
>       add_subdirectory(waffle_test)
>   endif()
>
> +add_subdirectory(utils)
>   add_subdirectory(waffle)
> diff --git a/src/utils/Android.mk b/src/utils/Android.mk
> new file mode 100644
> index 0000000..4bb067f
> --- /dev/null
> +++ b/src/utils/Android.mk
> @@ -0,0 +1,20 @@
> +LOCAL_PATH:= $(call my-dir)
> +include $(CLEAR_VARS)
> +
> +LOCAL_MODULE_TAGS := eng
> +LOCAL_MODULE:= wflinfo
> +
> +LOCAL_CFLAGS:= \
> +        -DANDROID_STUB \
> +        -DWAFFLE_HAS_ANDROID \
> +        -std=c99 \
> +
> +LOCAL_C_INCLUDES := $(LOCAL_PATH)/../include/waffle/
> +LOCAL_C_INCLUDES += $(LOCAL_PATH)/../src/waffle/

The headers inside src/waffle are private to libwaffle. Only the headers
in include/waffle are public. So src/waffle should be removed from the
INCLUDES path.

Did you test this on Android? If so, which lunch target and Android version?

> +
> +LOCAL_SRC_FILES:= \
> +    wflinfo.c \
> +
> +LOCAL_SHARED_LIBRARIES := libwaffle-1
> +
> +include $(BUILD_EXECUTABLE)
> diff --git a/src/utils/CMakeLists.txt b/src/utils/CMakeLists.txt
> new file mode 100644
> index 0000000..469077d
> --- /dev/null
> +++ b/src/utils/CMakeLists.txt
> @@ -0,0 +1,16 @@
> +
> +# ----------------------------------------------------------------------------
> +# Target: wflinfo (executable)
> +# ----------------------------------------------------------------------------
> +
> +add_executable(wflinfo wflinfo.c)
> +target_link_libraries(wflinfo ${waffle_libname})

In addition to installing this at $PREFIX/bin, it's also a good
idea, when building, to place it in $top_of_build_tree/bin rather
than in a deeply hidden CMake directory. To do that, you need
this CMake snippet:

set_target_properties(wflinfo
     PROPERTIES
         RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
     )

It's possible to set this project-wide in the toplevel CMakeLists.txt,
but as long as waffle has only a single utility I think it makes most
sense to set this property locally to wflinfo.

> +
> +if(waffle_on_mac)
> +    set_target_properties(wflinfo
> +        PROPERTIES
> +        COMPILE_FLAGS "-ObjC"
> +        )
> +endif()
> +
> +install(TARGETS wflinfo DESTINATION bin)
> diff --git a/src/utils/wflinfo.c b/src/utils/wflinfo.c
> new file mode 100644
> index 0000000..8b7f13b
> --- /dev/null
> +++ b/src/utils/wflinfo.c
> @@ -0,0 +1,642 @@
> +// Copyright 2013 Intel Corporation
> +//
> +// All rights reserved.
> +//
> +// Redistribution and use in source and binary forms, with or without
> +// modification, are permitted provided that the following conditions are met:
> +//
> +// - Redistributions of source code must retain the above copyright notice, this
> +//   list of conditions and the following disclaimer.
> +//
> +// - Redistributions in binary form must reproduce the above copyright notice,
> +//   this list of conditions and the following disclaimer in the documentation
> +//   and/or other materials provided with the distribution.
> +//
> +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
> +// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> +// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> +// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> +// 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.
> +
> +/// @file wflinfo.c
> +/// @brief Print GL info using Waffle.
> +///
> +/// This program does the following:
> +///     1. Dynamically choose the platform and GL API according to command
> +///        line arguments.
> +///     2. Create a GL context.
> +///     3. Print information about the context.
> +
> +#define _POSIX_C_SOURCE 199309L // glibc feature macro for nanosleep.

wflinfo doesn't use nanosleep, so you can likely remove this feature macro.

> +#define WAFFLE_API_VERSION 0x0103
> +#define WAFFLE_API_EXPERIMENTAL

Remove WAFFLE_API_EXPERIMENTAL. wflinfo uses no experimental api.

> +
> +#include <getopt.h>
> +#include <stdarg.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <time.h>

No nanosleep, so remove time.h.

> +
> +#ifdef __APPLE__
> +#    import <Foundation/NSAutoreleasePool.h>
> +#    import <Appkit/NSApplication.h>
> +
> +static void
> +removeXcodeArgs(int *argc, char **argv);
> +#endif
> +
> +#include "waffle.h"
> +
> +static const char *usage_message =
> +    "usage:\n"
> +    "    wflinfo --platform=android|cgl|gbm|glx|wayland|x11_egl\n"
> +    "             --api=gl|gles1|gles2|gles3\n"
> +    "             [--version=MAJOR.MINOR]\n"
> +    "             [--profile=core|compat|none]\n"
> +    "             [--forward-compatible]\n"
> +    "             [--debug]\n"
> +    "\n"
> +    "examples:\n"
> +    "    wflinfo --platform=glx --api=gl\n"
> +    "    wflinfo --platform=x11_egl --api=gl --version=3.2 --profile=core\n"
> +    "    wflinfo --platform=wayland --api=gles3\n"
> +    "\n"
> +    "description:\n"
> +    "    Create an OpenGL or GLES context and print information about it.\n"

Waffle's convention for user-visible strings that refer to GLES is to use the
api's full name: "OpenGL ES". See the difference between `git grep GLES`
and `git grep "OpenGL ES"`.

Eventually, the utility needs a manpage too. But the manpage can wait.

> +    "\n"
> +    "options:\n"
> +    "    --forward-compatible\n"
> +    "        Create a forward-compatible context.\n"
> +    "\n"
> +    "    --debug\n"
> +    "        Create a debug context.\n"
> +    ;
> +
> +enum {
> +    OPT_PLATFORM = 1,
> +    OPT_API,
> +    OPT_VERSION,
> +    OPT_PROFILE,
> +    OPT_DEBUG,
> +    OPT_FORWARD_COMPATIBLE,
> +};
> +
> +static const struct option get_opts[] = {
> +    { .name = "platform",       .has_arg = required_argument,     .val = OPT_PLATFORM },
> +    { .name = "api",            .has_arg = required_argument,     .val = OPT_API },
> +    { .name = "version",        .has_arg = required_argument,     .val = OPT_VERSION },
> +    { .name = "profile",        .has_arg = required_argument,     .val = OPT_PROFILE },
> +    { .name = "debug",          .has_arg = no_argument,           .val = OPT_DEBUG },
> +    { .name = "forward-compatible", .has_arg = no_argument,       .val = OPT_FORWARD_COMPATIBLE },
> +    { 0 },
> +};
> +
> +/// @defgroup Error handlers
> +/// @{
> +///
> +/// All error handlers exit.
> +///
> +
> +static void __attribute__((noreturn))
> +wflinfo_error(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    fflush(stdout);
> +
> +    va_start(ap, fmt);
> +    fprintf(stderr, "wflinfo: error: ");
> +    vfprintf(stderr, fmt, ap);
> +    fprintf(stderr, "\n");
> +    va_end(ap);
> +
> +    exit(EXIT_FAILURE);
> +}
> +static void __attribute__((noreturn))
> +usage_error_printf(const char *fmt, ...)
> +{
> +    fflush(stdout);
> +    fprintf(stderr, "wflinfo: usage error");
> +
> +    if (fmt) {
> +        va_list ap;
> +        va_start(ap, fmt);
> +        fprintf(stderr, ": ");
> +        vfprintf(stderr, fmt, ap);
> +        va_end(ap);
> +    }
> +
> +    fprintf(stderr, "\n");
> +    fprintf(stderr, "\n");
> +    fprintf(stderr, "%s", usage_message);
> +
> +    exit(EXIT_FAILURE);
> +}
> +
> +static void
> +error_waffle(void)
> +{
> +    const struct waffle_error_info *info = waffle_error_get_info();
> +    const char *code = waffle_error_to_string(info->code);
> +
> +    if (info->message_length > 0)
> +        wflinfo_error("%s: %s", code, info->message);
> +    else
> +        wflinfo_error("%s", code);
> +}
> +
> +static void
> +error_get_gl_symbol(const char *name)
> +{
> +    wflinfo_error("failed to get function pointer for %s", name);
> +}
> +
> +/// @}
> +/// @defgroup GL decalrations
> +/// @{
> +
> +typedef float GLclampf;
> +typedef unsigned int GLbitfield;
> +typedef unsigned int GLint;
> +typedef int GLsizei;
> +typedef unsigned int GLenum;
> +typedef void GLvoid;
> +typedef unsigned char GLubyte;
> +
> +enum {
> +    // Copied from <GL/gl*.h>.
> +    GL_UNSIGNED_BYTE =    0x00001401,
> +    GL_RGBA =             0x00001908,
> +    GL_COLOR_BUFFER_BIT = 0x00004000,

I believe the above three enums are unneeded.

> +
> +    GL_CONTEXT_FLAGS = 0x821e,
> +    GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT = 0x00000001,
> +    GL_CONTEXT_FLAG_DEBUG_BIT              = 0x00000002,
> +
> +    GL_VENDOR                              = 0x1F00,
> +    GL_RENDERER                            = 0x1F01,
> +    GL_VERSION                             = 0x1F02,
> +    GL_EXTENSIONS                          = 0x1F03,
> +    GL_NUM_EXTENSIONS                      = 0x821D,
> +};
> +
> +#define WINDOW_WIDTH  320
> +#define WINDOW_HEIGHT 240
> +
> +static GLenum (*glGetError)(void);
> +static void (*glGetIntegerv)(GLenum pname, GLint *params);
> +static const GLubyte * (*glGetString)(GLenum name);
> +static const GLubyte * (*glGetStringi)(GLenum name, GLint i);
> +
> +/// @}
> +/// @defgroup Parsing Options
> +/// @{
> +
> +struct options {
> +    /// @brief One of `WAFFLE_PLATFORM_*`.
> +    int platform;
> +
> +    /// @brief One of `WAFFLE_CONTEXT_OPENGL_*`.
> +    int context_api;
> +
> +    /// @brief One of `WAFFLE_CONTEXT_PROFILE_*` or `WAFFLE_NONE`.
> +    int context_profile;
> +
> +    int context_version;
> +
> +    bool context_forward_compatible;
> +    bool context_debug;
> +
> +    /// @brief One of `WAFFLE_DL_*`.
> +    int dl;
> +};
> +
> +struct enum_map {
> +    int i;
> +    const char *s;
> +};
> +
> +static const struct enum_map platform_map[] = {
> +    {WAFFLE_PLATFORM_ANDROID,   "android"       },
> +    {WAFFLE_PLATFORM_CGL,       "cgl",          },
> +    {WAFFLE_PLATFORM_GBM,       "gbm"           },
> +    {WAFFLE_PLATFORM_GLX,       "glx"           },
> +    {WAFFLE_PLATFORM_WAYLAND,   "wayland"       },
> +    {WAFFLE_PLATFORM_X11_EGL,   "x11_egl"       },
> +    {0,                         0               },
> +};
> +
> +static const struct enum_map context_api_map[] = {
> +    {WAFFLE_CONTEXT_OPENGL,         "gl"        },
> +    {WAFFLE_CONTEXT_OPENGL_ES1,     "gles1"     },
> +    {WAFFLE_CONTEXT_OPENGL_ES2,     "gles2"     },
> +    {WAFFLE_CONTEXT_OPENGL_ES3,     "gles3"     },
> +    {0,                             0           },
> +};
> +
> +/// @brief Translate string to `enum waffle_enum`.
> +///
> +/// @param self is a list of map items. The last item must be zero-filled.
> +/// @param result is altered only if @a s if found.
> +/// @return true if @a s was found in @a map.
> +static bool
> +enum_map_translate_str(
> +        const struct enum_map *self,
> +        const char *s,
> +        int *result)
> +{
> +    for (const struct enum_map *i = self; i->i != 0; ++i) {
> +        if (!strncmp(s, i->s, strlen(i->s) + 1)) {
> +            *result = i->i;
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> +
> +/// @return true on success.
> +static bool
> +parse_args(int argc, char *argv[], struct options *opts)
> +{
> +    bool ok;
> +    bool loop_get_opt = true;
> +
> +#ifdef __APPLE__
> +    removeXcodeArgs(&argc, argv);
> +#endif
> +
> +    // Set some context attrs to invalid values.
> +    opts->context_profile = -1;
> +    opts->context_version = -1;
> +
> +    while (loop_get_opt) {
> +        int opt = getopt_long(argc, argv, "", get_opts, NULL);
> +        switch (opt) {
> +            case -1:
> +                loop_get_opt = false;
> +                break;
> +            case '?':
> +                goto error_unrecognized_arg;
> +            case OPT_PLATFORM:
> +                ok = enum_map_translate_str(platform_map, optarg,
> +                                            &opts->platform);
> +                if (!ok) {
> +                    usage_error_printf("'%s' is not a valid platform",
> +                                       optarg);
> +                }
> +                break;
> +            case OPT_API:
> +                ok = enum_map_translate_str(context_api_map, optarg,
> +                                            &opts->context_api);
> +                if (!ok) {
> +                    usage_error_printf("'%s' is not a valid API for a GL "
> +                                       "context", optarg);
> +                }
> +                break;
> +            case OPT_VERSION: {
> +                int major;
> +                int minor;
> +                int match_count;
> +
> +                match_count = sscanf(optarg, "%d.%d", &major, &minor);
> +                if (match_count != 2) {
> +                    usage_error_printf("'%s' is not a valid GL version",
> +                                       optarg);
> +                }
> +                opts->context_version = 10 * major + minor;
> +                break;
> +            }
> +            case OPT_PROFILE:
> +                if (strcmp(optarg, "none") == 0) {
> +                    opts->context_profile = WAFFLE_NONE;
> +                } else if (strcmp(optarg, "core") == 0) {
> +                    opts->context_profile = WAFFLE_CONTEXT_CORE_PROFILE;
> +                } else if (strcmp(optarg, "compat") == 0) {
> +                    opts->context_profile = WAFFLE_CONTEXT_COMPATIBILITY_PROFILE;
> +                } else {
> +                    usage_error_printf("'%s' is not a valid GL profile",
> +                                       optarg);
> +                }
> +                break;
> +            case OPT_FORWARD_COMPATIBLE:
> +                opts->context_forward_compatible = true;
> +                break;
> +            case OPT_DEBUG:
> +                opts->context_debug = true;
> +                break;
> +            default:
> +                abort();
> +                loop_get_opt = false;
> +                break;
> +        }
> +    }
> +
> +    if (optind < argc) {
> +        goto error_unrecognized_arg;
> +    }
> +
> +    if (!opts->platform) {
> +        usage_error_printf("--platform is required");
> +    }
> +
> +    if (!opts->context_api) {
> +        usage_error_printf("--api is required");
> +    }
> +
> +    // Set dl.
> +    switch (opts->context_api) {
> +        case WAFFLE_CONTEXT_OPENGL:     opts->dl = WAFFLE_DL_OPENGL;      break;
> +        case WAFFLE_CONTEXT_OPENGL_ES1: opts->dl = WAFFLE_DL_OPENGL_ES1;  break;
> +        case WAFFLE_CONTEXT_OPENGL_ES2: opts->dl = WAFFLE_DL_OPENGL_ES2;  break;
> +        case WAFFLE_CONTEXT_OPENGL_ES3: opts->dl = WAFFLE_DL_OPENGL_ES3;  break;
> +        default:
> +            abort();
> +            break;
> +    }
> +
> +    return true;
> +
> +error_unrecognized_arg:
> +    usage_error_printf("unrecognized option '%s'", optarg);
> +}
> +
> +/// @}
> +
> +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
> +print_extensions_i(void)
> +{
> +    GLint count = 0, i;
> +    const char *ext;

Don't trust the GL. Be prepared for any GL call here to fail
and handle the failure gracefully.

> +
> +    glGetIntegerv(GL_NUM_EXTENSIONS, &count);
> +
> +    printf("OpenGL extensions:\n");
> +    for (i = 0; i < count; i++) {
> +        ext = (const char *) glGetStringi(GL_EXTENSIONS, i);
> +        printf("%s%s", ext, (i + 1) < count ? " " : "");
> +    }
> +    printf("\n");

As-is, this prints the following:
OpenGL extensions:
VERY_LONG_STRING

Not only does it look funny, but it the newline may confuse scripts that
need to parse wflinfo's output. To aid those scripts, I think this function
should print this:
OpenGL extensions: VERY_LONG_STRING

> +}
> +
> +/// @brief Print out information about the context that was created.
> +static bool
> +print_wflinfo(struct options *opts)
> +{
> +    const char *vendor = (const char *) glGetString(GL_VENDOR);
> +    const char *renderer = (const char *) glGetString(GL_RENDERER);
> +    const char *version_str = (const char *) glGetString(GL_VERSION);
> +    int version = parse_version(version_str);
> +

To correctly check if any of the above GL calls emitted an error, you must
first drain the the GL error state by repeatedly calling glGetError until it
returns GL_NO_ERROR. See the first 3 paragraphs of the OpenGL 4.3 Core spec,
Section 2.3 Command Execution.

> +    if (glGetError() != 0) {
> +        fprintf(stderr, "wflinfo: error occurred while retrieving GL information\n");

If you wish this function to proceed after an error, then it must handle the possibility
that any of the strings 'vendor', 'renderer', or 'version_str' may be NULL.

Alternatively, wflinfo could exit here on error. But I dislike that; it doesn't seem
sufficiently robust against unexpected behavior in quirky GL implementations.

> +    }
> +
> +    printf("OpenGL vendor string: %s\n", vendor);
> +    printf("OpenGL renderer string: %s\n", renderer);
> +    printf("OpenGL version string: %s\n", version_str);
> +
> +    if (opts->context_api == WAFFLE_CONTEXT_OPENGL && version >= 30) {
> +        print_extensions_i();
> +    } else {
> +        const char *extensions = (const char *) glGetString(GL_EXTENSIONS);

Don't trust the GL. Be prepared for glGetString to fail here.

> +        printf("OpenGL extensions:\n%s\n", extensions);
> +    }
> +
> +    return true;
> +}
> +
> +#ifdef __APPLE__
> +
> +static NSAutoreleasePool *pool;
> +
> +static void
> +cocoa_init(void)
> +{
> +    // From the NSApplication Class Reference:
> +    //     [...] if you do need to use Cocoa classes within the main()
> +    //     function itself (other than to load nib files or to instantiate
> +    //     NSApplication), you should create an autorelease pool before using
> +    //     the classes and then release the pool when you’re done.
> +    pool = [[NSAutoreleasePool alloc] init];
> +
> +    // From the NSApplication Class Reference:
> +    //     The sharedApplication class method initializes the display
> +    //     environment and connects your program to the window server and the
> +    //     display server.
> +    //
> +    // It also creates the singleton NSApp if it does not yet exist.
> +    [NSApplication sharedApplication];
> +}
> +
> +static void
> +cocoa_finish(void)
> +{
> +    [pool drain];
> +}
> +
> +static void
> +removeArg(int index, int *argc, char **argv)
> +{
> +    --*argc;
> +    for (; index < *argc; ++index)
> +        argv[index] = argv[index + 1];
> +}
> +
> +static void
> +removeXcodeArgs(int *argc, char **argv)
> +{
> +    // Xcode sometimes adds additional arguments.
> +    for (int i = 1; i < *argc; )
> +    {
> +        if (strcmp(argv[i], "-NSDocumentRevisionsDebugMode") == 0 ||
> +            strcmp(argv[i], "-ApplePersistenceIgnoreState" ) == 0)
> +        {
> +            removeArg(i, argc, argv);
> +            removeArg(i, argc, argv);
> +        } else
> +            ++i;
> +    }
> +}
> +
> +#endif // __APPLE__
> +
> +int
> +main(int argc, char **argv)
> +{
> +    bool ok;
> +    int i;
> +
> +    struct options opts = {0};
> +
> +    int32_t init_attrib_list[3];
> +    int32_t config_attrib_list[64];
> +
> +    struct waffle_display *dpy;
> +    struct waffle_config *config;
> +    struct waffle_context *ctx;
> +    struct waffle_window *window;
> +
> +    #ifdef __APPLE__
> +        cocoa_init();
> +    #endif
> +
> +    ok = parse_args(argc, argv, &opts);
> +    if (!ok)
> +        exit(EXIT_FAILURE);
> +
> +    i = 0;
> +    init_attrib_list[i++] = WAFFLE_PLATFORM;
> +    init_attrib_list[i++] = opts.platform;
> +    init_attrib_list[i++] = WAFFLE_NONE;
> +
> +    ok = waffle_init(init_attrib_list);
> +    if (!ok)
> +        error_waffle();
> +
> +    dpy = waffle_display_connect(NULL);
> +    if (!dpy)
> +        error_waffle();
> +
> +    if (!waffle_display_supports_context_api(dpy, opts.context_api)) {
> +        wflinfo_error("Display does not support %s",
> +                       waffle_enum_to_string(opts.context_api));
> +    }
> +
> +    glGetError = waffle_dl_sym(opts.dl, "glGetError");
> +    if (!glGetError)
> +        error_get_gl_symbol("glGetError");
> +
> +    glGetIntegerv = waffle_dl_sym(opts.dl, "glGetIntegerv");
> +    if (!glGetIntegerv)
> +        error_get_gl_symbol("glGetIntegerv");
> +
> +    glGetString = waffle_dl_sym(opts.dl, "glGetString");
> +    if (!glGetString)
> +        error_get_gl_symbol("glGetString");
> +
> +    glGetStringi = waffle_dl_sym(opts.dl, "glGetStringi");
> +    if (!glGetStringi && opts.context_api != WAFFLE_CONTEXT_OPENGL_ES1)
> +        error_get_gl_symbol("glGetStringi");

There are errors in the path for glGetStringi.

glGetStringi does not exist in ES2 or in GL versions earlier than 3.0. Therfore
wflinfo fails too often here.

Trying to decide here when and when not to get the glGetStringi symbol would be
messy. I suggest you move this snippet inside the appropriate if-branch within
print_wflinfo().

Also, at least for GL >= 3.0, the correct method to obtain the glGetStringi
symbol in a vendor-independent manner is to use waffle_get_proc_address().
The rules for ES are more complicated, but also irrelevant because you use
glGetStringi only for GL.

> +
> +    i = 0;
> +    config_attrib_list[i++] = WAFFLE_CONTEXT_API;
> +    config_attrib_list[i++] = opts.context_api;
> +
> +    if (opts.context_profile != -1) {
> +        config_attrib_list[i++] = WAFFLE_CONTEXT_PROFILE;
> +        config_attrib_list[i++] = opts.context_profile;
> +    }
> +
> +    if (opts.context_version != -1) {
> +        config_attrib_list[i++] = WAFFLE_CONTEXT_MAJOR_VERSION;
> +        config_attrib_list[i++] = opts.context_version / 10;
> +        config_attrib_list[i++] = WAFFLE_CONTEXT_MINOR_VERSION;
> +        config_attrib_list[i++] = opts.context_version % 10;
> +    }
> +
> +    if (opts.context_forward_compatible) {
> +        config_attrib_list[i++] = WAFFLE_CONTEXT_FORWARD_COMPATIBLE;
> +        config_attrib_list[i++] = true;
> +    }
> +
> +    if (opts.context_debug) {
> +        config_attrib_list[i++] = WAFFLE_CONTEXT_DEBUG;
> +        config_attrib_list[i++] = true;
> +    }
> +
> +    config_attrib_list[i++] = WAFFLE_RED_SIZE;
> +    config_attrib_list[i++] = 8;
> +    config_attrib_list[i++] = WAFFLE_GREEN_SIZE;
> +    config_attrib_list[i++] = 8;
> +    config_attrib_list[i++] = WAFFLE_BLUE_SIZE;
> +    config_attrib_list[i++] = 8;
> +    config_attrib_list[i++] = WAFFLE_DOUBLE_BUFFERED;
> +    config_attrib_list[i++] = true;
> +    config_attrib_list[i++] = 0;

wflinfo's goal is to make the context current under the widest possible
circumstances. Accordingly, it should choose the most lenient config filter.
To do that, set each of WAFFLE_{RED,GREEN,BLUE,ALPHA,DEPTH,STENCIL}_SIZE
and WAFFLE_DOUBLE_BUFFERED to WAFFLE_DONT_CARE.

FYI, the default values for these attributes are listed on the manpage
waffle_config_choose(3) [http://people.freedesktop.org/~chadversary/waffle/man/waffle_config.3.html].

> +
> +    config = waffle_config_choose(dpy, config_attrib_list);
> +    if (!config)
> +        error_waffle();
> +
> +    ctx = waffle_context_create(config, NULL);
> +    if (!ctx)
> +        error_waffle();
> +
> +    window = waffle_window_create(config, WINDOW_WIDTH, WINDOW_HEIGHT);
> +    if (!window)
> +        error_waffle();

It's disappointing that wflinfo creates a window to make the context current.
But, that *is* the most direct and consistent way to do it, so your method
looks good to me.

> +
> +    ok = waffle_make_current(dpy, window, ctx);
> +    if (!ok)
> +        error_waffle();
> +
> +    GLint context_flags = 0;
> +    if (opts.context_forward_compatible || opts.context_debug) {
> +        glGetIntegerv(GL_CONTEXT_FLAGS, &context_flags);
> +    }
> +
> +    if (opts.context_forward_compatible
> +        && !(context_flags & GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT)) {
> +        wflinfo_error("context is not forward-compatible");
> +    }
> +
> +    if (opts.context_debug
> +        && !(context_flags & GL_CONTEXT_FLAG_DEBUG_BIT)) {
> +        wflinfo_error("context is not a debug context");
> +    }

Since wflinfo is intended to a user-facing utility, similar to glxinfo,
I think the above error messages should be a little more verbose. Something
to the effect of "Requested a forward-compatible context, but the actual
context is not forward-compatible because GL_CONTEXT_FLAGS lacks the
GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT".

It may seem like I'm nit picking, but I've had several people complain that
Waffle's error messages, and Piglit's too, are too terse for people not familiar with
Waffle's codebase.

> +
> +    ok = print_wflinfo(&opts);
> +    if (!ok)
> +        error_waffle();
> +
> +    ok = waffle_window_destroy(window);
> +    if (!ok)
> +        error_waffle();
> +
> +    ok = waffle_context_destroy(ctx);
> +    if (!ok)
> +        error_waffle();
> +
> +    ok = waffle_config_destroy(config);
> +    if (!ok)
> +        error_waffle();
> +
> +    ok = waffle_display_disconnect(dpy);
> +    if (!ok)
> +        error_waffle();
> +
> +    #ifdef __APPLE__
> +        cocoa_finish();
> +    #endif
> +
> +    return EXIT_SUCCESS;
> +}
>



More information about the waffle mailing list