[waffle] [PATCH v2 4/4] wflinfo.c: Add a --json flag that prints json
Emil Velikov
emil.l.velikov at gmail.com
Wed Jan 6 14:06:24 PST 2016
Hi Dylan,
Perhaps I'm not the best person to comment on the JSON side of things
(esp since Frank sent related series a few hours ago), then again a
few things, that might be interesting.
On 5 January 2016 at 19:46, <baker.dylan.c at gmail.com> wrote:
> From: Dylan Baker <baker.dylan.c at gmail.com>
>
> This adds some code to print a JSON formatted version of data provided
> by wflinfo.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
>
> v2: - Add -j short options (Frank)
> - Make context flags always an array (Chad)
> - Remove explicit returns from void functions (Chad)
> - Fix spaces around "*" in variable definitions
> - Nest waffle specific information in a waffle hash and OpenGL
> specific information in an OpenGL hash
> - use ARRAY_SIZE macro
> - rebase on previous changes
> ---
> src/utils/wflinfo.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 176 insertions(+), 2 deletions(-)
>
> diff --git a/src/utils/wflinfo.c b/src/utils/wflinfo.c
> index 8ee95c6..456f91b 100644
> --- a/src/utils/wflinfo.c
> +++ b/src/utils/wflinfo.c
> @@ -85,6 +85,9 @@ static const char *usage_message =
> " --debug-context\n"
> " Create a debug context.\n"
> "\n"
> + " -j, --json\n"
> + " Print a JSON formatted summary.\n"
> + "\n"
> " -h, --help\n"
> " Print wflinfo usage information.\n"
> "\n"
> @@ -104,6 +107,7 @@ enum {
> OPT_VERBOSE = 'v',
> OPT_DEBUG_CONTEXT,
> OPT_FORWARD_COMPATIBLE,
> + OPT_JSON = 'j',
> OPT_HELP = 'h',
> };
>
> @@ -115,6 +119,7 @@ static const struct option get_opts[] = {
> { .name = "verbose", .has_arg = no_argument, .val = OPT_VERBOSE },
> { .name = "debug-context", .has_arg = no_argument, .val = OPT_DEBUG_CONTEXT },
> { .name = "forward-compatible", .has_arg = no_argument, .val = OPT_FORWARD_COMPATIBLE },
> + { .name = "json", .has_arg = no_argument, .val = OPT_JSON },
> { .name = "help", .has_arg = no_argument, .val = OPT_HELP },
> { 0 },
> };
> @@ -252,6 +257,8 @@ struct options {
>
> bool verbose;
>
> + bool json;
> +
> bool context_forward_compatible;
> bool context_debug;
>
> @@ -337,7 +344,7 @@ parse_args(int argc, char *argv[], struct options *opts)
> opterr = 0;
>
> while (loop_get_opt) {
> - int opt = getopt_long(argc, argv, "a:hp:vV:", get_opts, NULL);
> + int opt = getopt_long(argc, argv, "a:hp:vV:j", get_opts, NULL);
> switch (opt) {
> case -1:
> loop_get_opt = false;
> @@ -395,6 +402,9 @@ parse_args(int argc, char *argv[], struct options *opts)
> case OPT_DEBUG_CONTEXT:
> opts->context_debug = true;
> break;
> + case OPT_JSON:
> + opts->json = true;
> + break;
> case OPT_HELP:
> write_usage_and_exit(stdout, EXIT_SUCCESS);
> break;
> @@ -563,6 +573,165 @@ print_context_flags(void)
> printf("\n");
> }
>
> +static void
> +json_print_context_flags(void)
> +{
> + const int flag_count = ARRAY_SIZE(context_flags);
const unsigned flag_count ?
> + GLint gl_context_flags = 0;
> +
> + printf(" \"context flags\": ");
> +
> + glGetIntegerv(GL_CONTEXT_FLAGS, &gl_context_flags);
> + if (glGetError() != GL_NO_ERROR) {
> + printf("\"WFLINFO_GL_ERROR\"\n");
> + return;
> + }
> +
> + if (gl_context_flags == 0) {
> + printf("[]");
Missing \n ?
> + return;
> + }
> +
> + printf("[\n");
> + for (int i = 0; i < flag_count; i++) {
unsigned i ?
> + if ((context_flags[i].flag & gl_context_flags) != 0) {
> + printf(" \"%s\"", context_flags[i].str);
> + gl_context_flags = gl_context_flags & ~context_flags[i].flag;
> + if (i != flag_count) {
This always evaluates to true, doesn't it ?
> + printf(",\n");
> + }
> + }
> + }
> + for (int i = 0; gl_context_flags != 0; gl_context_flags >>= 1, i++) {
> + if ((gl_context_flags & 1) != 0) {
> + printf(",\n");
We'll get two consecutive ",\n" considering the one above ?
> + printf(" 0x%x", 1 << i);
... and we're missing the \n for the last entry.
Easier way might be to do "leading" \n's ?
> + }
> + }
> + printf(" ]");
> +}
> +
> +static void
> +json_print_extensions(bool use_stringi)
> +{
> + // Print extensions in JSON format
> + printf(" \"extensions\": [\n");
> + if (use_stringi) {
> + GLint count = 0, i;
> + const char *ext;
> +
> + glGetIntegerv(GL_NUM_EXTENSIONS, &count);
> + if (glGetError() != GL_NO_ERROR) {
> + printf("\"WFLINFO_GL_ERROR\"");
Intentional difference in tabulation between previous function, here
and else statement ?
> + } else {
> + for (i = 0; i < count; i++) {
> + ext = (const char *) glGetStringi(GL_EXTENSIONS, i);
> + if (glGetError() != GL_NO_ERROR)
> + ext = "WFLINFO_GL_ERROR";
> + printf(" \"%s\"%s\n", ext, (i + 1) < count ? "," : "");
> + }
> + }
> + } else {
> + char *extensions = (char *) glGetString(GL_EXTENSIONS);
> + if (glGetError() != GL_NO_ERROR) {
> + printf(" \"WFLINFO_GL_ERROR\"");
> + } else {
> + char *splitter = strtok(extensions, " ");
> +
Note to self: do we care if a driver returns a single extension
(and/or empty extensions string) ?
> + // The JSON doesn't strictly need to be newline seperated, but
> + // it makes it much easier to read. Split the lines and add commas correctly
> + while(true) {
> + printf(" \"%s\"", splitter);
> + splitter = strtok(NULL, " ");
> +
> + if (splitter != NULL) {
> + printf(",\n");
> + } else {
> + break;
> + }
> + }
> + }
> + printf("\n");
> + }
> + printf(" ]\n");
> +}
> +
> +/// @brief Print JSON formatted OpenGL (ES) information
> +static bool
> +print_json(const struct options *opts)
> +{
> + while(glGetError() != GL_NO_ERROR) {
> + /* Clear all errors */
Space between while and (... then again why the while loop ?
> + }
> +
> + const char *vendor = get_vendor();
> + const char *renderer = get_renderer();
> + const char *version_str = get_version();
> +
> + const char *platform = enum_map_to_str(platform_map, opts->platform);
> + assert(platform != NULL);
> +
> + const char *api = enum_map_to_str(context_api_map, opts->context_api);
> + assert(api != NULL);
> +
> + printf("{\n");
> + printf(" \"waffle\": {\n");
> + printf(" \"platform\": \"%s\",\n", platform);
> + printf(" \"api\": \"%s\"\n", api);
> + printf(" },\n");
> + printf(" \"OpenGL\": {\n");
> + printf(" \"vendor\": \"%s\",\n", vendor);
> + printf(" \"renderer\": \"%s\",\n", renderer);
> + printf(" \"version\": \"%s\"", version_str);
> + // No comma is allowed on the last element. If this context doesn't have
> + // context flags or verbose is not requested this will be the last element.
> +
> + int version = parse_version(version_str);
> +
> + if (opts->context_api == WAFFLE_CONTEXT_OPENGL && version >= 31) {
> + // Add the comma and newline before the context flags.
> + printf(",\n");
> + json_print_context_flags();
> + }
> +
> + // OpenGL and OpenGL ES >= 3.0 support glGetStringi(GL_EXTENSION, i).
> + const bool use_getstringi = version >= 30;
> +
> + if (!glGetStringi && use_getstringi)
> + error_get_gl_symbol("glGetStringi");
> +
> + if (opts->verbose) {
> + // Add the command and newline
> + printf(",\n");
> +
> + // See the equivalent section in print_wflinfo() for information about
> + // this hunk
> + const char *language_str = "None";
> + if ((opts->context_api == WAFFLE_CONTEXT_OPENGL && version >= 20) ||
> + opts->context_api == WAFFLE_CONTEXT_OPENGL_ES2 ||
> + opts->context_api == WAFFLE_CONTEXT_OPENGL_ES3) {
Align the last two ones - o (in opts) should be the same column as the (
> + language_str = (const char *) glGetString(GL_SHADING_LANGUAGE_VERSION);
> + if (glGetError() != GL_NO_ERROR || language_str == NULL) {
> + language_str = "WFLINFO_GL_ERROR";
> + }
> + }
> +
> + printf(" \"shading language version\": \"%s\",\n", language_str);
> +
> + json_print_extensions(use_getstringi);
> +
> + } else {
> + // If verbose isn't being used, add a newline so that the closing curly
> + // brace will be nicely formatted
> + printf("\n");
> + }
> +
> + printf(" }\n");
> + printf("}\n");
> +
> + return true;
Afaict you can make the function return void ?
Seems that most (all?) of the comments here apply to the existing code
in wflinfo.c where this was inspired/copied from. As mentioned by
others - pondering on reusing the existing functions... I'll give it a
quick test tomorrow.
A bit tired so do take my comments with a grain of salt.
That said (with or without the suggestions in 1 and 3) patches 1-3 are
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
-Emil
More information about the waffle
mailing list