Re: [waffle] [PATCH v2 0/8] nacl backend implementation

2015-02-12 Thread Tapani Pälli 



On 02/12/2015 03:45 AM, Chad Versace wrote:

On 02/09/2015 05:24 AM, Tapani Pälli wrote:

Here's v2 of the nacl backend implementation patches. I've gone through
review comments and hopefully did not forget anything. I changed the way
how swapbuffers is implemented and now it blocks (so no event required).

All patches are in tree here:
http://cgit.freedesktop.org/~tpalli/waffle/log/?h=nacl

Alternative tree with extra patch to have naclgears testcase:
http://cgit.freedesktop.org/~tpalli/waffle/log/?h=naclgears

Thanks;

Tapani Pälli (8):
   nacl: add attrib_list parameter to create_window
   nacl: add supports_context_api implementation
   nacl: add implementation for waffle_config_choose
   nacl: add implementation for waffle_context_create
   nacl: add implementation for window create and resize
   nacl: add implementation for waffle_make_current
   nacl: add implementation for waffle_window_swap_buffers
   nacl: add implementation for waffle_dl_sym


Tapani,
This entire series, including v3 of the swap buffers patch, is
Reviewed-by: Chad Versace chad.vers...@intel.com

I don't fully understand the NaCl APIs in the swap buffers patch,
but I'll just have to live with that.

Should I pull from your 'nacl' branch or collect the patches off the list?



Thanks, they match so you are free to do whichever is easier to go with!

You can forward any bugs reported on this backend for me, I'll also be 
testing this with new nacl SDK's and adding gles3 support whenever it 
happens.


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

Re: [waffle] [PATCH v2 8/8] nacl: add implementation for waffle_dl_sym

2015-02-12 Thread Emil Velikov 
On 9 February 2015 at 13:24, Tapani Pälli tapani.pa...@intel.com wrote:
 v2: use wcore_calloc, code cleanup (Emil Velikov)
 open dll only once in can_open (Chad Versace)

 Signed-off-by: Tapani Pälli tapani.pa...@intel.com
 ---
  src/waffle/nacl/nacl_platform.c | 69 
 +++--
  src/waffle/nacl/nacl_platform.h |  1 +
  2 files changed, 68 insertions(+), 2 deletions(-)

 diff --git a/src/waffle/nacl/nacl_platform.c b/src/waffle/nacl/nacl_platform.c
 index c4fefe5..22169da 100644
 --- a/src/waffle/nacl/nacl_platform.c
 +++ b/src/waffle/nacl/nacl_platform.c
 @@ -43,6 +43,11 @@ nacl_platform_destroy(struct wcore_platform *wc_self)

  nacl_teardown(self-nacl);

 +if (self-gl_dl)
 +if (dlclose(self-gl_dl) != 0)
 +wcore_errorf(WAFFLE_ERROR_UNKNOWN, dlclose failed: %s,
 + dlerror());
 +
  free(self);
  return ok;
  }
 @@ -51,7 +56,41 @@ static bool
  nacl_platform_dl_can_open(struct wcore_platform *wc_self,
int32_t waffle_dl)
  {
 -return false;
 +struct nacl_platform *self = nacl_platform(wc_self);
 +
 +switch (waffle_dl) {
 +case WAFFLE_DL_OPENGL_ES2:
 +if (!self-gl_dl)
 +self-gl_dl = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY);
 +break;
 +// API not supported
 +default:
 +return false;
 +}
 +
 +if (!self-gl_dl)
 +wcore_errorf(WAFFLE_ERROR_UNKNOWN, dlopen failed: %s, dlerror());
 +
 +return self-gl_dl ? true : false;
 +}
 +
 +// Construct a string that maps GL function to NaCl function
 +// by concating given prefix and function name tail from 'src'.
 +static char *
 +nacl_prefix(const char *src, const char *prefix)
 +{
 +if (strncmp(src, gl, 2) != 0)
 +return NULL;
 +
 +uint32_t len = strlen(src) + strlen(prefix);
 +
 +char *dst = wcore_calloc(len);
 +if (!dst)
 +return NULL;
 +
 +snprintf(dst, len, %s%s, prefix, src + 2);
 +
 +return dst;
  }

  static void*
 @@ -59,7 +98,33 @@ nacl_platform_dl_sym(struct wcore_platform *wc_self,
   int32_t waffle_dl,
   const char *name)
  {
 -return NULL;
 +struct nacl_platform *self = nacl_platform(wc_self);
 +char *nacl_name = NULL;
 +void *func = NULL;
 +
 +if (!self-gl_dl)
 +if (!nacl_platform_dl_can_open(wc_self, waffle_dl))
I'm sorry boring this again but - doing this will not set the correct
error on WAFFLE_DL_OPENGL_ES3.

 +return false;
return NULL;

Cheers,
Emil
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle

[waffle] [PATCH] cgl: do not emit an error on dl_can_open

2015-02-12 Thread Emil Velikov 
According to the documentation and the linux backend one should not emit
an error but simply return true/false.

Signed-off-by: Emil Velikov emil.l.veli...@gmail.com
---
 src/waffle/cgl/cgl_dl.m | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/waffle/cgl/cgl_dl.m b/src/waffle/cgl/cgl_dl.m
index 2d13067..764ecf0 100644
--- a/src/waffle/cgl/cgl_dl.m
+++ b/src/waffle/cgl/cgl_dl.m
@@ -77,8 +77,13 @@ cgl_dl_can_open(struct wcore_platform *wc_plat,
 int32_t waffle_dl)
 {
 struct cgl_platform *plat = cgl_platform(wc_plat);
+bool ok;
 
-if (!cgl_dl_check_enum(waffle_dl))
+WCORE_ERROR_DISABLED({
+ok = cgl_dl_check_enum(waffle_dl);
+});
+
+if (!ok)
 return false;
 
 if (plat-dl_gl != NULL)
-- 
2.2.2

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

Re: [waffle] [wflinfo] [RFC] platform-specific info from wflinfo

2015-02-12 Thread Emil Velikov 
On 12 February 2015 at 02:01, Chad Versace chad.vers...@intel.com wrote:
 On 02/10/2015 01:20 PM, Frank Henigman wrote:
 On Tue, Feb 10, 2015 at 4:08 PM, Frank Henigman fjhenig...@google.com 
 wrote:

 Looks like Issue #3 is the format of the information.  I thought it
 was given we should duplicate existing glxinfo/eglinfo/etc as closely
 as possible, in order to be a drop-in replacement, but if I follow the
 suggestions Chad made on github
 (https://github.com/fjhenigman/waffle/commit/d0b45bb9850e6ae29ee379a2d3e8ba14afc1b872)
 we'll be diverging.  Improving on existing tools is ok with me - I
 don't have a huge investment in code to parse their output - but I
 wonder if others feel differently.

 (+Jordan, +Dylan, questions below)

 Oh, when I made those Github comments, I didn't know you were trying to
 duplicate glxinfo output verbatim. Now I understand why the GLX lines
 look so different from wflinfo's current output.

 glxinfo wraps long lines for extension strings and separates extension names 
 with commas.
 wflinfo intentionally prints extensions strings in their original form: 
 single line,
 extension names separated by spaces. If I recall correctly, Jordan and Dylan 
 wanted
 that format so that consumers who parsed wflinfo text output would be 
 guaranteed a stable
 format.

 If wflinfo has mixed line formats (some lines are comma-separated and 
 wrapped, some
 are space-separated), I fear that may cause problems for already-existing 
 consumers.
 Dylan, Jordan, do you have an opinion here? Does this really matter?

The above are some examples why I am doubtful about adding such
function in waffle.

One might want the extensions listed in alphabetic order, another to
have them one per line, another will likely be interested the client
extensions, or maybe the server ones, the GLX/CGL/WGL/EGL version only
?

Imho in order for one to get some flexibility I would opt for query
mechanism for issue 1.

Chad, genuine question, can you please describe how having a string is
more extensible ?


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

Re: [waffle] [PATCH v2 3/8] nacl: add implementation for waffle_config_choose

2015-02-12 Thread Emil Velikov 
On 9 February 2015 at 13:24, Tapani Pälli tapani.pa...@intel.com wrote:
 Patch fills attributes table suitable for Pepper API from
 wcore_config_attrs passed by the application.

 v2: throw error on unsupported context type (Chad Versace)
 code cleanup, comment for max attribs (Emil Velikov)

 Signed-off-by: Tapani Pälli tapani.pa...@intel.com
 ---
  src/waffle/nacl/nacl_config.c | 37 +
  src/waffle/nacl/nacl_config.h |  1 +
  2 files changed, 38 insertions(+)

 diff --git a/src/waffle/nacl/nacl_config.c b/src/waffle/nacl/nacl_config.c
 index 27a75e1..7478493 100644
 --- a/src/waffle/nacl/nacl_config.c
 +++ b/src/waffle/nacl/nacl_config.c
 @@ -23,7 +23,9 @@
  // 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 ppapi/c/pp_graphics_3d.h
  #include nacl_config.h
 +#include wcore_error.h

  bool
  nacl_config_destroy(struct wcore_config *wc_self)
 @@ -50,6 +52,41 @@ nacl_config_choose(struct wcore_platform *wc_plat,
  if (self == NULL)
  return NULL;

 +// Currently only OpenGL ES 2.0 is supported.
 +if (attrs-context_api != WAFFLE_CONTEXT_OPENGL_ES2) {
 +wcore_errorf(WAFFLE_ERROR_UNSUPPORTED_ON_PLATFORM,
 + NaCl does no support context type %s.,
 + wcore_enum_to_string(attrs-context_api));
We're leaking self - free(self);

Feel free to have this as a follow up patch.

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

Re: [waffle] EGL null platform

2015-02-12 Thread Frank Henigman 
EGL_PLATFORM_NULL is not upstreamed yet either, though perhaps it
could be fairly easily.
Also I'm developing with minigbm, though it could probably also work
with mesa gbm.
My priority is to get this working for chromeos tests, as we've
already switched to null platform + minigbm.
That will probably take a couple weeks, after which I'll be keen to upstream.
No doubt some early review will be beneficial, so as soon as the
branch is somewhat stable I'll invite the list to take a look.

On Wed, Feb 11, 2015 at 9:05 PM, Chad Versace chad.vers...@intel.com wrote:
 Frank, I see in your Github repo that you're working on support for
 Chrome OS's null platform. I'm looking forward to it, because some
 of us in Intel would like to use it too. Do you have an idea when
 it will be ready for upstreaming?


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

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

Re: [waffle] [PATCH v3] nacl: add implementation for waffle_window_swap_buffers

2015-02-12 Thread Tapani Pälli 



On 02/12/2015 03:47 AM, Chad Versace wrote:

On 02/09/2015 06:22 AM, Tapani Pälli wrote:

Implementation for nacl is somewhat different as for other platforms,
platform needs to ensure that the previous swap has finished before
issuing GL commands or more swaps. This is done by introducing a worker
thread that does buffer swaps from a work queue and uses a semaphore to
lock main thread until swap is finished.

v2: add error messaging if pp::Graphics3D::SwapBuffers fails
 add semaphore to implement blocking swap()
 remove extra nacl_swapbuffers() c++ function (Chad, Emil)

v3: destroy semaphore when thread dies

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/waffle/nacl/nacl_container.cpp | 12 +
  src/waffle/nacl/nacl_container.h   |  2 +-
  src/waffle/nacl/nacl_swap_thread.h | 94 ++
  src/waffle/nacl/nacl_window.c  |  3 +-
  4 files changed, 109 insertions(+), 2 deletions(-)
  create mode 100644 src/waffle/nacl/nacl_swap_thread.h

diff --git a/src/waffle/nacl/nacl_container.cpp 
b/src/waffle/nacl/nacl_container.cpp
index fe907ff..84ab1da 100644
--- a/src/waffle/nacl/nacl_container.cpp
+++ b/src/waffle/nacl/nacl_container.cpp
@@ -28,11 +28,13 @@
  #include ppapi/cpp/module.h
  #include ppapi/c/pp_errors.h
  #include nacl_container.h
+#include nacl_swap_thread.h

  namespace waffle {

  struct nacl_container {
  pp::Graphics3D *ctx;
+NaclSwapThread *swapper;

  void *glapi;
  bool (*glInitializePPAPI) (PPB_GetInterface);
@@ -49,6 +51,7 @@ nacl_container_dtor(waffle::nacl_container *nc)
  if (nc-glapi)
  dlclose(nc-glapi);

+delete nc-swapper;
  delete nc;
  }

@@ -119,6 +122,7 @@ nacl_context_init(waffle::nacl_container *nc, struct 
nacl_config *cfg)
  return false;
  }

+nc-swapper = new NaclSwapThread(pp_instance, nc-ctx);
  return true;
  }

@@ -194,3 +198,11 @@ nacl_makecurrent(nacl_container *nc, bool release)

  return true;
  }
+
+extern C bool
+nacl_swapbuffers(nacl_container *nc)
+{
+waffle::nacl_container *cpp_nc =
+reinterpret_castwaffle::nacl_container*(nc);
+return cpp_nc-swapper-swap();
+}
diff --git a/src/waffle/nacl/nacl_container.h b/src/waffle/nacl/nacl_container.h
index ca26a1f..579856d 100644
--- a/src/waffle/nacl/nacl_container.h
+++ b/src/waffle/nacl/nacl_container.h
@@ -43,7 +43,7 @@ bool nacl_context_init(struct nacl_container *nc, struct 
nacl_config *cfg);
  bool nacl_resize(struct nacl_container *nc, int32_t width, int32_t height);
  bool nacl_makecurrent(struct nacl_container *nc, bool release);
  void nacl_context_fini(struct nacl_container *nc);
-
+bool nacl_swapbuffers(struct nacl_container *nc);
  #ifdef __cplusplus
  };
  #endif
diff --git a/src/waffle/nacl/nacl_swap_thread.h 
b/src/waffle/nacl/nacl_swap_thread.h
new file mode 100644
index 000..8e8687b
--- /dev/null
+++ b/src/waffle/nacl/nacl_swap_thread.h
@@ -0,0 +1,94 @@
+// Copyright 2014 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.
+
+#include ppapi/cpp/graphics_3d.h
+#include ppapi/cpp/instance.h
+#include ppapi/utility/completion_callback_factory.h
+#include ppapi/utility/threading/simple_thread.h
+#include wcore_error.h
+#include semaphore.h
+
+// Thread takes care that we do not issue another buffer
+// swap before previous swap has completed.
+class NaclSwapThread : public pp::SimpleThread
+{
+public:
+explicit NaclSwapThread(const pp::InstanceHandle instance,
+pp::Graphics3D *_ctx) :
+pp::SimpleThread(instance),
+ctx(_ctx),
+cbf(this)
+{
+Start();
+sem_init(sem, 0, 0);
+}
+
+

Re: [waffle] [wflinfo] [RFC] platform-specific info from wflinfo

2015-02-12 Thread Frank Henigman 
On Thu, Feb 12, 2015 at 5:44 AM, Emil Velikov emil.l.veli...@gmail.com wrote:
 On 12 February 2015 at 02:01, Chad Versace chad.vers...@intel.com wrote:
 On 02/10/2015 01:20 PM, Frank Henigman wrote:
 On Tue, Feb 10, 2015 at 4:08 PM, Frank Henigman fjhenig...@google.com 
 wrote:

 Looks like Issue #3 is the format of the information.  I thought it
 was given we should duplicate existing glxinfo/eglinfo/etc as closely
 as possible, in order to be a drop-in replacement, but if I follow the
 suggestions Chad made on github
 (https://github.com/fjhenigman/waffle/commit/d0b45bb9850e6ae29ee379a2d3e8ba14afc1b872)
 we'll be diverging.  Improving on existing tools is ok with me - I
 don't have a huge investment in code to parse their output - but I
 wonder if others feel differently.

 (+Jordan, +Dylan, questions below)

 Oh, when I made those Github comments, I didn't know you were trying to
 duplicate glxinfo output verbatim. Now I understand why the GLX lines
 look so different from wflinfo's current output.

 glxinfo wraps long lines for extension strings and separates extension names 
 with commas.
 wflinfo intentionally prints extensions strings in their original form: 
 single line,
 extension names separated by spaces. If I recall correctly, Jordan and Dylan 
 wanted
 that format so that consumers who parsed wflinfo text output would be 
 guaranteed a stable
 format.

 If wflinfo has mixed line formats (some lines are comma-separated and 
 wrapped, some
 are space-separated), I fear that may cause problems for already-existing 
 consumers.
 Dylan, Jordan, do you have an opinion here? Does this really matter?

 The above are some examples why I am doubtful about adding such
 function in waffle.

 One might want the extensions listed in alphabetic order, another to
 have them one per line, another will likely be interested the client
 extensions, or maybe the server ones, the GLX/CGL/WGL/EGL version only
 ?

 Imho in order for one to get some flexibility I would opt for query
 mechanism for issue 1.

 Chad, genuine question, can you please describe how having a string is
 more extensible ?


 Thanks
 Emil

I'm sold on easy parsing and wflinfo compatibility over {glx,egl}info
compatibility.  There's no reason we can't have everything actually.
The parameter I proposed could be used to select output format: match
glxinfo, match older wflinfo, latest and easiest-to-parse, etc.
I don't mean to answer for Chad, but the nice thing about returning a
string is we can tweak it without changing the api, and maybe without
breaking existing parsers (at least not badly).  You could say this is
just a fuzzy and therefore useless kind of api, but that's glass half
empty thinking.  (^:
I think a string is a good start.  We can lock in the format(s) if and
when we choose, it in no way hinders later attempts at something
fancier, and it shouldn't be hard to keep the string for compatibility
after something fancier is available.
___
waffle mailing list
waffle@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH v2 8/8] nacl: add implementation for waffle_dl_sym

2015-02-12 Thread Tapani Pälli 



On 02/12/2015 01:02 PM, Emil Velikov wrote:

On 9 February 2015 at 13:24, Tapani Pälli tapani.pa...@intel.com wrote:

v2: use wcore_calloc, code cleanup (Emil Velikov)
 open dll only once in can_open (Chad Versace)

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/waffle/nacl/nacl_platform.c | 69 +++--
  src/waffle/nacl/nacl_platform.h |  1 +
  2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/waffle/nacl/nacl_platform.c b/src/waffle/nacl/nacl_platform.c
index c4fefe5..22169da 100644
--- a/src/waffle/nacl/nacl_platform.c
+++ b/src/waffle/nacl/nacl_platform.c
@@ -43,6 +43,11 @@ nacl_platform_destroy(struct wcore_platform *wc_self)

  nacl_teardown(self-nacl);

+if (self-gl_dl)
+if (dlclose(self-gl_dl) != 0)
+wcore_errorf(WAFFLE_ERROR_UNKNOWN, dlclose failed: %s,
+ dlerror());
+
  free(self);
  return ok;
  }
@@ -51,7 +56,41 @@ static bool
  nacl_platform_dl_can_open(struct wcore_platform *wc_self,
int32_t waffle_dl)
  {
-return false;
+struct nacl_platform *self = nacl_platform(wc_self);
+
+switch (waffle_dl) {
+case WAFFLE_DL_OPENGL_ES2:
+if (!self-gl_dl)
+self-gl_dl = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY);
+break;
+// API not supported
+default:
+return false;
+}
+
+if (!self-gl_dl)
+wcore_errorf(WAFFLE_ERROR_UNKNOWN, dlopen failed: %s, dlerror());
+
+return self-gl_dl ? true : false;
+}
+
+// Construct a string that maps GL function to NaCl function
+// by concating given prefix and function name tail from 'src'.
+static char *
+nacl_prefix(const char *src, const char *prefix)
+{
+if (strncmp(src, gl, 2) != 0)
+return NULL;
+
+uint32_t len = strlen(src) + strlen(prefix);
+
+char *dst = wcore_calloc(len);
+if (!dst)
+return NULL;
+
+snprintf(dst, len, %s%s, prefix, src + 2);
+
+return dst;
  }

  static void*
@@ -59,7 +98,33 @@ nacl_platform_dl_sym(struct wcore_platform *wc_self,
   int32_t waffle_dl,
   const char *name)
  {
-return NULL;
+struct nacl_platform *self = nacl_platform(wc_self);
+char *nacl_name = NULL;
+void *func = NULL;
+
+if (!self-gl_dl)
+if (!nacl_platform_dl_can_open(wc_self, waffle_dl))

I'm sorry boring this again but - doing this will not set the correct
error on WAFFLE_DL_OPENGL_ES3.


Thanks for pointing this out. I've been now busy with some bugs but I'll 
promise to fix this.




+return false;

return NULL;

Cheers,
Emil




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