Re: [waffle] json, approach 2, version 2

[Chad Versace]

On Thu 21 Apr 2016, Frank Henigman wrote:
> Thanks Emil and Chad for reviewing my json series.  All suggestions
> implemented in v2, except where I replied inline.  I'll hold off
> sending in case there's more back-and-forth over the first set of
> comments.  Would also be nice if Chad merged his get-current branch
> into master, as I use it in v2.

> Not sure if I did the right thing with glx info.  Seems like all three
> sections (server, client, common) show about the same list of
> extensions.  That can wait until I send v2, or if anyone wants to look
> now:
> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7

Hi Frank,

I've returned from sabbatical, and am now trying to catch up with
everything.

I can't find the above sha1 in your repo. Should I be examining your
json5 branch?
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 00/29] Rework the functionality test(s)

[Chad Versace]

On Tue 26 Apr 2016, Emil Velikov wrote:
> Humble ping ?

And an embarassedly late merge.

Thanks for all the cleanups. All the patches in the series, except patch
1, are merged. I skipped patch 1 because it introduced a single symbol,
but that symbol was never used.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 00/13] Do less validation of the context version

[Chad Versace]

On Tue 26 Apr 2016, Emil Velikov wrote:
> Humble ping ?

And an embarrasedly late reply...

This year, I've taken nearly 4 months off (paternity leave + vacation
+ sabbatical) and was in a Vulkan crunch for the 3 months before that.
My schedule has finally returned to normal.

> On 5 April 2016 at 22:58, Emil Velikov  wrote:
> > Hi all,
> >
> > This is a re-spin of an ancient RFC [1] covering two (core) topics
> >  - Should waffle do fine grained checking of the context version prior
> > to feeding it to the driver ? Leaning towards no.

After considering this more, I agree with you. Waffle shouldn't add an
extra layer of validation on top of the GL/GLES/GLX/EGL drivers, except
when the validation is truly helpful.

> >  - Should we rely upon the library (libGL/libGLESv1/libGLESv2) presence
> > to determine if context of respective API is supported ? Same sentiment.

I agree. Strictly speaking, the presence of libGLFoo does not indicate
the availability of the GLFoo API.

However, patch 7 of your series regresses the gl_basic GLES1 tests. I'm
investigating that now. Maybe the bug is Waffle's, maybe Mesa's; I'm
unsure.

> >
> > And last but not least I've thrown out a bunch of the
> > wcore_error_internal() in favour of assert. Core already handles all of
> > those 'default' cases, thus we can simplify things ;-)

Yes, it does make the code cleaner.

I've submitted the first 6 patches to Intel's CI. I'll merge if there
are no regressions.

I'm holding off on patches 7 and above until I diagnose the regressions.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 00/29] Rework the functionality test(s)

[Emil Velikov]

On 14 June 2016 at 18:10, Chad Versace  wrote:
> On Tue 26 Apr 2016, Emil Velikov wrote:
>> Humble ping ?
>
> And an embarassedly late merge.
>
> Thanks for all the cleanups. All the patches in the series, except patch
> 1, are merged. I skipped patch 1 because it introduced a single symbol,
> but that symbol was never used.

I'm slightly leaning towards having that symbol, as with it (and a gbm
cleanup) one should be able to run the waffle tests on Travis CI
(which has old wayland). I won't insist on it though.
Alternatively we should really bump the minimum required wayland version to 1.4.

Which one would you prefer ?

Thanks
Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 4/4] gbm: implement window_resize

[Emil Velikov]

On 16 May 2016 at 11:48, Emil Velikov  wrote:
> From: Emil Velikov 
>
> Reuse the existing init/teardown functions to create a new window and
> nuke the old one on success.
>
> Note: as we need the original config, we keep a reference to it in
> struct wcore_config. Ideally there will be a generic way to query it.
>
> Signed-off-by: Emil Velikov 
> ---
>  src/waffle/gbm/wgbm_platform.c |  1 +
>  src/waffle/gbm/wgbm_window.c   | 43 
> ++
>  src/waffle/gbm/wgbm_window.h   |  5 +
>  3 files changed, 49 insertions(+)
>
> diff --git a/src/waffle/gbm/wgbm_platform.c b/src/waffle/gbm/wgbm_platform.c
> index 5e36534..f4147af 100644
> --- a/src/waffle/gbm/wgbm_platform.c
> +++ b/src/waffle/gbm/wgbm_platform.c
> @@ -210,6 +210,7 @@ static const struct wcore_platform_vtbl 
> wgbm_platform_vtbl = {
>  .destroy = wgbm_window_destroy,
>  .show = wgbm_window_show,
>  .swap_buffers = wgbm_window_swap_buffers,
> +.resize = wgbm_window_resize,
>  .get_native = wgbm_window_get_native,
>  },
>  };
> diff --git a/src/waffle/gbm/wgbm_window.c b/src/waffle/gbm/wgbm_window.c
> index 12ae197..d9dcba0 100644
> --- a/src/waffle/gbm/wgbm_window.c
> +++ b/src/waffle/gbm/wgbm_window.c
> @@ -32,8 +32,10 @@
>
>  #include "wcore_attrib_list.h"
>  #include "wcore_error.h"
> +#include "wcore_tinfo.h"
>
>  #include "wegl_config.h"
> +#include "wegl_util.h"
>
>  #include "wgbm_config.h"
>  #include "wgbm_display.h"
> @@ -92,6 +94,7 @@ wgbm_window_init(struct wgbm_window *self,
>  if (!ok)
>  return false;
>
> +self->wc_config = wc_config;
>  return true;
>  }
>
> @@ -155,6 +158,46 @@ wgbm_window_swap_buffers(struct wcore_window *wc_self)
>  return true;
>  }
>
> +bool
> +wgbm_window_resize(struct wcore_window *wc_self,
> +   int32_t width, int32_t height)
> +{
> +struct wcore_display *wc_dpy = wc_self->display;
> +struct wcore_platform *wc_plat = wc_self->display->platform;
> +struct wgbm_window *self = wgbm_window(wc_self);
> +struct wgbm_window backup_self;
> +struct wcore_context *wc_ctx;
> +struct wcore_tinfo *tinfo;
> +bool ok = true;
> +
> +// Backup the old window/surface so that we can restore it upon failure.
> +backup_self = *self;
> +
> +ok = wgbm_window_init(self, wc_plat, self->wc_config, width, height);
> +if (!ok)
> +goto error;
> +
> +tinfo = wcore_tinfo_get();
> +wc_ctx = tinfo->current_context;
> +
> +// XXX: Can/should we use waffle_make_current() here ?
> +ok = wegl_make_current(wc_plat, wc_dpy, wc_self, wc_ctx);
> +if (!ok)
> +goto error;
> +
> +// We don't need to set current_display or current_window
> +tinfo->current_context = wc_ctx;
> +
Self note: in 99.99% of the time we won't need the above line. Thus we
could drop the XXX: comment a couple of lines before it.

Note for all: I'm wondering if some drivers won't have odd behaviour
if waffle_window_resize() is called where wc_ctx = NULL. Not 100%
familiar if the spec ensures that things are sane.

Any ideas ?

Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

[waffle] [PATCH 1/2] tests/gl_basic: Make GL symbol queries more robust

[Chad Versace]

The rules that dictate how to properly query a GL symbol are complex.
The rules depend on the OS, on the winsys API, and even on the
particular driver being used. The rules differ between EGL 1.4 and EGL
1.5; differ between Linux, Windows, and Mac; and differ between Mesa and
Mali.

gl_basic_test didn't try very hard when querying a GL symbol. It tried
only dlsym. If dlsym failed, then the test failed.

Make gl_basic_test more robust by falling back to
waffle_get_proc_address() whenever dlsym fails.

Suggested-by: Emil Velikov 
Signed-off-by: Chad Versace 
---
 tests/functional/gl_basic_test.c | 55 +---
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/tests/functional/gl_basic_test.c b/tests/functional/gl_basic_test.c
index 7af2229..4a90dfd 100644
--- a/tests/functional/gl_basic_test.c
+++ b/tests/functional/gl_basic_test.c
@@ -169,19 +169,36 @@ teardown(void **state)
 return 0;
 }
 
-static int32_t
-libgl_from_context_api(int32_t waffle_context_api)
+// The rules that dictate how to properly query a GL symbol are complex. The
+// rules depend on the OS, on the winsys API, and even on the particular driver
+// being used. The rules differ between EGL 1.4 and EGL 1.5; differ between
+// Linux, Windows, and Mac; and differ between Mesa and Mali.
+//
+// This function hides that complexity with a naive heuristic: try, then try
+// again.
+static void *
+get_gl_symbol(enum waffle_enum context_api, const char *name)
 {
-switch (waffle_context_api) {
-case WAFFLE_CONTEXT_OPENGL: return WAFFLE_DL_OPENGL;
-case WAFFLE_CONTEXT_OPENGL_ES1: return WAFFLE_DL_OPENGL_ES1;
-case WAFFLE_CONTEXT_OPENGL_ES2: return WAFFLE_DL_OPENGL_ES2;
-case WAFFLE_CONTEXT_OPENGL_ES3: return WAFFLE_DL_OPENGL_ES3;
-
-default:
-assert_true(0);
-return 0;
+void *sym = NULL;
+enum waffle_enum dl = 0;
+
+switch (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: assert_true(0); break;
+}
+
+if (waffle_dl_can_open(dl)) {
+sym = waffle_dl_sym(dl, name);
 }
+
+if (!sym) {
+sym = waffle_get_proc_address(name);
+}
+
+return sym;
 }
 
 static int
@@ -270,8 +287,6 @@ gl_basic_draw__(void **state, struct gl_basic_draw_args__ 
args)
 bool context_debug = args.debug;
 bool alpha = args.alpha;
 
-int32_t libgl;
-
 int32_t config_attrib_list[64];
 int i;
 
@@ -281,8 +296,6 @@ gl_basic_draw__(void **state, struct gl_basic_draw_args__ 
args)
 0,
 };
 
-libgl = libgl_from_context_api(waffle_context_api);
-
 i = 0;
 config_attrib_list[i++] = WAFFLE_CONTEXT_API;
 config_attrib_list[i++] = waffle_context_api;
@@ -353,12 +366,12 @@ gl_basic_draw__(void **state, struct gl_basic_draw_args__ 
args)
 }
 
 // Get OpenGL functions.
-assert_true(glClear = waffle_dl_sym(libgl, "glClear"));
-assert_true(glClearColor= waffle_dl_sym(libgl, "glClearColor"));
-assert_true(glGetError  = waffle_dl_sym(libgl, "glGetError"));
-assert_true(glGetIntegerv   = waffle_dl_sym(libgl, "glGetIntegerv"));
-assert_true(glReadPixels= waffle_dl_sym(libgl, "glReadPixels"));
-assert_true(glGetString = waffle_dl_sym(libgl, "glGetString"));
+assert_true(glClear = get_gl_symbol(waffle_context_api, 
"glClear"));
+assert_true(glClearColor= get_gl_symbol(waffle_context_api, 
"glClearColor"));
+assert_true(glGetError  = get_gl_symbol(waffle_context_api, 
"glGetError"));
+assert_true(glGetIntegerv   = get_gl_symbol(waffle_context_api, 
"glGetIntegerv"));
+assert_true(glReadPixels= get_gl_symbol(waffle_context_api, 
"glReadPixels"));
+assert_true(glGetString = get_gl_symbol(waffle_context_api, 
"glGetString"));
 
 assert_true(waffle_make_current(ts->dpy, ts->window, ts->ctx));
 
-- 
2.9.0.rc2

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 1/2] tests/gl_basic: Make GL symbol queries more robust

[Emil Velikov]

On 14 June 2016 at 22:22, Chad Versace  wrote:
> Again, with Emil's real email.
>
> On Tue 14 Jun 2016, Chad Versace wrote:
>> Emil, this patch fixes the regressions caused by your patch (still
>> uncommitted):
>>
>>   [PATCH 07/13] wegl: untangle dl_can_open() and support_api()
>>
Fortunate me did not see the issue since I have both libGLESv1_CM and
libGLESv2 libraries :-)

That aside:
Reviewed-by: Emil Velikov 

Thanks
Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 1/2] tests/gl_basic: Make GL symbol queries more robust

[Chad Versace]

Emil, this patch fixes the regressions caused by your patch (still
uncommitted):

  [PATCH 07/13] wegl: untangle dl_can_open() and support_api()

On Tue 14 Jun 2016, Chad Versace wrote:
> The rules that dictate how to properly query a GL symbol are complex.
> The rules depend on the OS, on the winsys API, and even on the
> particular driver being used. The rules differ between EGL 1.4 and EGL
> 1.5; differ between Linux, Windows, and Mac; and differ between Mesa and
> Mali.
> 
> gl_basic_test didn't try very hard when querying a GL symbol. It tried
> only dlsym. If dlsym failed, then the test failed.
> 
> Make gl_basic_test more robust by falling back to
> waffle_get_proc_address() whenever dlsym fails.
> 
> Suggested-by: Emil Velikov 
> Signed-off-by: Chad Versace 
> ---
>  tests/functional/gl_basic_test.c | 55 
> +---
>  1 file changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/functional/gl_basic_test.c 
> b/tests/functional/gl_basic_test.c
> index 7af2229..4a90dfd 100644
> --- a/tests/functional/gl_basic_test.c
> +++ b/tests/functional/gl_basic_test.c
> @@ -169,19 +169,36 @@ teardown(void **state)
>  return 0;
>  }
>  
> -static int32_t
> -libgl_from_context_api(int32_t waffle_context_api)
> +// The rules that dictate how to properly query a GL symbol are complex. The
> +// rules depend on the OS, on the winsys API, and even on the particular 
> driver
> +// being used. The rules differ between EGL 1.4 and EGL 1.5; differ between
> +// Linux, Windows, and Mac; and differ between Mesa and Mali.
> +//
> +// This function hides that complexity with a naive heuristic: try, then try
> +// again.
> +static void *
> +get_gl_symbol(enum waffle_enum context_api, const char *name)
>  {
> -switch (waffle_context_api) {
> -case WAFFLE_CONTEXT_OPENGL: return WAFFLE_DL_OPENGL;
> -case WAFFLE_CONTEXT_OPENGL_ES1: return WAFFLE_DL_OPENGL_ES1;
> -case WAFFLE_CONTEXT_OPENGL_ES2: return WAFFLE_DL_OPENGL_ES2;
> -case WAFFLE_CONTEXT_OPENGL_ES3: return WAFFLE_DL_OPENGL_ES3;
> -
> -default:
> -assert_true(0);
> -return 0;
> +void *sym = NULL;
> +enum waffle_enum dl = 0;
> +
> +switch (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: assert_true(0); break;
> +}
> +
> +if (waffle_dl_can_open(dl)) {
> +sym = waffle_dl_sym(dl, name);
>  }
> +
> +if (!sym) {
> +sym = waffle_get_proc_address(name);
> +}
> +
> +return sym;
>  }
>  
>  static int
> @@ -270,8 +287,6 @@ gl_basic_draw__(void **state, struct gl_basic_draw_args__ 
> args)
>  bool context_debug = args.debug;
>  bool alpha = args.alpha;
>  
> -int32_t libgl;
> -
>  int32_t config_attrib_list[64];
>  int i;
>  
> @@ -281,8 +296,6 @@ gl_basic_draw__(void **state, struct gl_basic_draw_args__ 
> args)
>  0,
>  };
>  
> -libgl = libgl_from_context_api(waffle_context_api);
> -
>  i = 0;
>  config_attrib_list[i++] = WAFFLE_CONTEXT_API;
>  config_attrib_list[i++] = waffle_context_api;
> @@ -353,12 +366,12 @@ gl_basic_draw__(void **state, struct 
> gl_basic_draw_args__ args)
>  }
>  
>  // Get OpenGL functions.
> -assert_true(glClear = waffle_dl_sym(libgl, "glClear"));
> -assert_true(glClearColor= waffle_dl_sym(libgl, "glClearColor"));
> -assert_true(glGetError  = waffle_dl_sym(libgl, "glGetError"));
> -assert_true(glGetIntegerv   = waffle_dl_sym(libgl, "glGetIntegerv"));
> -assert_true(glReadPixels= waffle_dl_sym(libgl, "glReadPixels"));
> -assert_true(glGetString = waffle_dl_sym(libgl, "glGetString"));
> +assert_true(glClear = get_gl_symbol(waffle_context_api, 
> "glClear"));
> +assert_true(glClearColor= get_gl_symbol(waffle_context_api, 
> "glClearColor"));
> +assert_true(glGetError  = get_gl_symbol(waffle_context_api, 
> "glGetError"));
> +assert_true(glGetIntegerv   = get_gl_symbol(waffle_context_api, 
> "glGetIntegerv"));
> +assert_true(glReadPixels= get_gl_symbol(waffle_context_api, 
> "glReadPixels"));
> +assert_true(glGetString = get_gl_symbol(waffle_context_api, 
> "glGetString"));
>  
>  assert_true(waffle_make_current(ts->dpy, ts->window, ts->ctx));
>  
> -- 
> 2.9.0.rc2
> 
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 1/2] tests/gl_basic: Make GL symbol queries more robust

[Chad Versace]

Again, with Emil's real email.

On Tue 14 Jun 2016, Chad Versace wrote:
> Emil, this patch fixes the regressions caused by your patch (still
> uncommitted):
> 
>   [PATCH 07/13] wegl: untangle dl_can_open() and support_api()
> 
> On Tue 14 Jun 2016, Chad Versace wrote:
> > The rules that dictate how to properly query a GL symbol are complex.
> > The rules depend on the OS, on the winsys API, and even on the
> > particular driver being used. The rules differ between EGL 1.4 and EGL
> > 1.5; differ between Linux, Windows, and Mac; and differ between Mesa and
> > Mali.
> > 
> > gl_basic_test didn't try very hard when querying a GL symbol. It tried
> > only dlsym. If dlsym failed, then the test failed.
> > 
> > Make gl_basic_test more robust by falling back to
> > waffle_get_proc_address() whenever dlsym fails.
> > 
> > Suggested-by: Emil Velikov 
> > Signed-off-by: Chad Versace 
> > ---
> >  tests/functional/gl_basic_test.c | 55 
> > +---
> >  1 file changed, 34 insertions(+), 21 deletions(-)
> > 
> > diff --git a/tests/functional/gl_basic_test.c 
> > b/tests/functional/gl_basic_test.c
> > index 7af2229..4a90dfd 100644
> > --- a/tests/functional/gl_basic_test.c
> > +++ b/tests/functional/gl_basic_test.c
> > @@ -169,19 +169,36 @@ teardown(void **state)
> >  return 0;
> >  }
> >  
> > -static int32_t
> > -libgl_from_context_api(int32_t waffle_context_api)
> > +// The rules that dictate how to properly query a GL symbol are complex. 
> > The
> > +// rules depend on the OS, on the winsys API, and even on the particular 
> > driver
> > +// being used. The rules differ between EGL 1.4 and EGL 1.5; differ between
> > +// Linux, Windows, and Mac; and differ between Mesa and Mali.
> > +//
> > +// This function hides that complexity with a naive heuristic: try, then 
> > try
> > +// again.
> > +static void *
> > +get_gl_symbol(enum waffle_enum context_api, const char *name)
> >  {
> > -switch (waffle_context_api) {
> > -case WAFFLE_CONTEXT_OPENGL: return WAFFLE_DL_OPENGL;
> > -case WAFFLE_CONTEXT_OPENGL_ES1: return WAFFLE_DL_OPENGL_ES1;
> > -case WAFFLE_CONTEXT_OPENGL_ES2: return WAFFLE_DL_OPENGL_ES2;
> > -case WAFFLE_CONTEXT_OPENGL_ES3: return WAFFLE_DL_OPENGL_ES3;
> > -
> > -default:
> > -assert_true(0);
> > -return 0;
> > +void *sym = NULL;
> > +enum waffle_enum dl = 0;
> > +
> > +switch (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: assert_true(0); break;
> > +}
> > +
> > +if (waffle_dl_can_open(dl)) {
> > +sym = waffle_dl_sym(dl, name);
> >  }
> > +
> > +if (!sym) {
> > +sym = waffle_get_proc_address(name);
> > +}
> > +
> > +return sym;
> >  }
> >  
> >  static int
> > @@ -270,8 +287,6 @@ gl_basic_draw__(void **state, struct 
> > gl_basic_draw_args__ args)
> >  bool context_debug = args.debug;
> >  bool alpha = args.alpha;
> >  
> > -int32_t libgl;
> > -
> >  int32_t config_attrib_list[64];
> >  int i;
> >  
> > @@ -281,8 +296,6 @@ gl_basic_draw__(void **state, struct 
> > gl_basic_draw_args__ args)
> >  0,
> >  };
> >  
> > -libgl = libgl_from_context_api(waffle_context_api);
> > -
> >  i = 0;
> >  config_attrib_list[i++] = WAFFLE_CONTEXT_API;
> >  config_attrib_list[i++] = waffle_context_api;
> > @@ -353,12 +366,12 @@ gl_basic_draw__(void **state, struct 
> > gl_basic_draw_args__ args)
> >  }
> >  
> >  // Get OpenGL functions.
> > -assert_true(glClear = waffle_dl_sym(libgl, "glClear"));
> > -assert_true(glClearColor= waffle_dl_sym(libgl, "glClearColor"));
> > -assert_true(glGetError  = waffle_dl_sym(libgl, "glGetError"));
> > -assert_true(glGetIntegerv   = waffle_dl_sym(libgl, "glGetIntegerv"));
> > -assert_true(glReadPixels= waffle_dl_sym(libgl, "glReadPixels"));
> > -assert_true(glGetString = waffle_dl_sym(libgl, "glGetString"));
> > +assert_true(glClear = get_gl_symbol(waffle_context_api, 
> > "glClear"));
> > +assert_true(glClearColor= get_gl_symbol(waffle_context_api, 
> > "glClearColor"));
> > +assert_true(glGetError  = get_gl_symbol(waffle_context_api, 
> > "glGetError"));
> > +assert_true(glGetIntegerv   = get_gl_symbol(waffle_context_api, 
> > "glGetIntegerv"));
> > +assert_true(glReadPixels= get_gl_symbol(waffle_context_api, 
> > "glReadPixels"));
> > +assert_true(glGetString = get_gl_symbol(waffle_context_api, 
> > "glGetString"));
> >  
> >  assert_true(waffle_make_current(ts->dpy, ts->window, ts->ctx));
> >  
> > -- 
> > 2.9.0.rc2
> > 

Re: [waffle] json, approach 2, version 2

[Chad Versace]

On Thu 21 Apr 2016, Frank Henigman wrote:
> Thanks Emil and Chad for reviewing my json series.  All suggestions
> implemented in v2, except where I replied inline.  I'll hold off
> sending in case there's more back-and-forth over the first set of
> comments.  Would also be nice if Chad merged his get-current branch
> into master, as I use it in v2.

Hi Frank, I think now is a good time to send v2.

Since the patches are not on the list yet, I'll say a few comments here:

core: add JSON library
Overall, looks good. The only problem I see is
a potential buffer overflow in put().

waffle: add waffle_display_info_json()
For simplicity's sake, please drop the 'platform_too'
parameter to waffle_display_info_json(). Waffle should
return all the info, and the client the should ignore
any keys (such as platform-specific keys) that it does
not recognize.

The patch has some problems regarding "current-ness".
It says the new function "returns a JSON string
containing information about the current context on the
given display". As defined by EGL and GLX, though,
current-ness is a mapping of (thread) -> (display,
context, surfaces), and not a mapping of
(thread, display) -> (context, surfaces).

As a consequence, the waffle_display_info_json()
signature is incorrect because due to its 'display'
parameter. If the function has a display parameter, then
that allows the user to do undefined things such as
below:

struct waffle_display *dpy1 = ...;
struct waffle_display *ctx1 = ...;

struct waffle_display *dpy2 = ...;
struct waffle_display *ctx2 = ...;

waffle_make_current(dpy1, NULL, ctx1);
// use ctx1
waffle_make_current(dpy2, NULL, ctx2);
// use ctx2

// Now dpy2 and ctx2 are bound to the thread.
// So what should waffle_display_info_json() do
// when given dpy1?
waffle_display_info_json(dpy1);

wflinfo: another option for JSON output
As far as I can tell, the two JSON formats are identical
except for formatting, as they should be :). Since
they're identical, I don't understand the need to add
the new format 'json2'.

At the end of this patch series, your json is Waffle's
*real* json. So I expected this patch to remove Dylan's
json code from wflinfo.c and replace it with yours. Then
--format=json would return your json.

> When comparing my json output to the landed json output I noticed that
> the landed version omits the context flags found in the old format.
> Was that deliberate?  If so I'll remove it from my json.

It's an accident that wflinfo does not currently report the context
flags. I'd like to see them in the wflinfo output.

> Not sure if I did the right thing with glx info.  Seems like all three
> sections (server, client, common) show about the same list of
> extensions.  That can wait until I send v2, or if anyone wants to look
> now:
> https://github.com/fjhenigman/waffle/commit/b358ac50c00ce51fae6546b1e96c9adc32fcbdc7

As Emil said earlier: wflinfo should report all three sections, as they
do sometimes differ.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 06/12] wflinfo: add option for JSON output

[Chad Versace]

On Thu 21 Apr 2016, Frank Henigman wrote:
> On Fri, Apr 8, 2016 at 8:14 PM, Chad Versace  wrote:
> > On 01/06/2016 11:56 AM, Frank Henigman wrote:

> I really hope we can keep this simple.
> 
> I mentioned in the cover letter to this series that the next step is
> to gut wflinfo and translate json to the old format.  We can insert
> that after this patch if you like.  Option 1 is the nicest by far, if
> the translator is written in python.  It's just a few lines of code to
> read the json, plus a table that lists the correspondence between old
> format lines and json keys.  So we have some really simple C code to
> call the waffle api and get the json, and a really simple python
> script to translate to old format.  I've got this much roughed out.
> For maximum backward compatibility these two bits would be tied
> together by a script that accepts all the options currently accepted
> by wflinfo, i.e. we replace wflinfo with a script.  Which for
> portability is also written in python.  Yes this introduces a
> dependency on python to run wflinfo, but I doubt that's much of a
> problem.

I'm open to exploring that path. I just took a serious look at
wflinfo.c, and it's needless complex for the little job that it does. If
you could port it to Python, greatly reducing and simplifying the code,
I'd like to see that.

I've never used Python's C interface, so I look forward to learning
about it from your patches.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle