Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 02/03/2015 08:26 PM, Chad Versace wrote: On 02/03/2015 07:37 AM, Emil Velikov wrote: On 23 January 2015 at 07:59, Tapani Pälli wrote: Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. Signed-off-by: Tapani Pälli --- [...] @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) { if (!nc) return; + + nc->ctx = pp::Graphics3D(); I would guess that you want to nuke the Graphics3D ctx first ? +nc->glSetCurrentContextPPAPI(0); +nc->glTerminatePPAPI(); + Imho the teardown should be symmetrical to the setup - i.e. create a new function nacl_context_fini (or similar) which has the above three calls, and gets executed in nacl_context_destroy. I just want to echo Emil here. Writing teardown to be symmteric to setup is generally a good idea. OK, will do. +static bool +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) +{ [...] + +dlclose(glapi); + Calling the function pointers after closing the handle causes segfaults on my Archlinux machine. Am I the only one or does nacl has something special in this regard ? Hmmm... This seems wrong to me too. Huh yes, this is wrong. I believe it works currently only because the functions resolved do not allocate memory ... maybe. I will move dlclose to happen when teardown happens. // Tapani ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 02/03/2015 07:37 AM, Emil Velikov wrote: > On 23 January 2015 at 07:59, Tapani Pälli wrote: >> Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. >> >> Signed-off-by: Tapani Pälli >> --- > [...] > >> @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) >> { >> if (!nc) >> return; >> + > + nc->ctx = pp::Graphics3D(); > I would guess that you want to nuke the Graphics3D ctx first ? > >> +nc->glSetCurrentContextPPAPI(0); >> +nc->glTerminatePPAPI(); >> + > Imho the teardown should be symmetrical to the setup - i.e. create a > new function nacl_context_fini (or similar) which has the above three > calls, and gets executed in nacl_context_destroy. I just want to echo Emil here. Writing teardown to be symmteric to setup is generally a good idea. > >> +static bool >> +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) >> +{ > [...] >> + >> +dlclose(glapi); >> + > Calling the function pointers after closing the handle causes > segfaults on my Archlinux machine. Am I the only one or does nacl has > something special in this regard ? Hmmm... This seems wrong to me too. signature.asc Description: OpenPGP digital signature ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 3 February 2015 at 15:37, Emil Velikov wrote: > On 23 January 2015 at 07:59, Tapani Pälli wrote: >> Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. >> >> Signed-off-by: Tapani Pälli >> --- > [...] > >> @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) >> { >> if (!nc) >> return; >> + > + nc->ctx = pp::Graphics3D(); > I would guess that you want to nuke the Graphics3D ctx first ? > >> +nc->glSetCurrentContextPPAPI(0); >> +nc->glTerminatePPAPI(); >> + > Imho the teardown should be symmetrical to the setup - i.e. create a > new function nacl_context_fini (or similar) which has the above three > calls, and gets executed in nacl_context_destroy. > Completely forgot - some of the func ptrs can be NULL. > +static bool > +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) [...] > +if (!pp_instance->BindGraphics(nc->ctx)) { > +wcore_errorf(WAFFLE_ERROR_FATAL, "Unable to bind 3D context.\n"); > +nc->ctx = pp::Graphics3D(); > +nc->glSetCurrentContextPPAPI(0); As nacl_context_fini comes along you might want to drop the above two calls. -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 23 January 2015 at 07:59, Tapani Pälli wrote: > Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. > > Signed-off-by: Tapani Pälli > --- [...] > @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) > { > if (!nc) > return; > + + nc->ctx = pp::Graphics3D(); I would guess that you want to nuke the Graphics3D ctx first ? > +nc->glSetCurrentContextPPAPI(0); > +nc->glTerminatePPAPI(); > + Imho the teardown should be symmetrical to the setup - i.e. create a new function nacl_context_fini (or similar) which has the above three calls, and gets executed in nacl_context_destroy. > +static bool > +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) > +{ [...] > + > +dlclose(glapi); > + Calling the function pointers after closing the handle causes segfaults on my Archlinux machine. Am I the only one or does nacl has something special in this regard ? > diff --git a/src/waffle/nacl/nacl_container.h > b/src/waffle/nacl/nacl_container.h > index 61d935c..81472cc 100644 > --- a/src/waffle/nacl/nacl_container.h > +++ b/src/waffle/nacl/nacl_container.h > @@ -25,13 +25,21 @@ > > #ifdef __cplusplus > > +#include > + > extern "C" { > #endif > > +#include "nacl_config.h" > +#include "wcore_error.h" > + > +#define NACL_GLES2_LIBRARY "libppapi_gles2.so" > + Please move the above hunk to nacl_container.c. -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 02/03/2015 01:16 AM, Chad Versace wrote: On 01/22/2015 11:59 PM, Tapani Pälli wrote: Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. Signed-off-by: Tapani Pälli --- src/waffle/nacl/nacl_container.cpp | 81 ++ src/waffle/nacl/nacl_container.h | 8 src/waffle/nacl/nacl_context.c | 6 +++ 3 files changed, 95 insertions(+) diff --git a/src/waffle/nacl/nacl_container.cpp b/src/waffle/nacl/nacl_container.cpp index 3e8073c..e352da9 100644 --- a/src/waffle/nacl/nacl_container.cpp +++ b/src/waffle/nacl/nacl_container.cpp @@ -24,12 +24,18 @@ // 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/cpp/module.h" #include "nacl_container.h" namespace waffle { struct nacl_container { pp::Graphics3D ctx; + +bool (*glInitializePPAPI) (PPB_GetInterface); +void (*glSetCurrentContextPPAPI) (PP_Resource); +bool (*glTerminatePPAPI) (void); }; static void @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) { if (!nc) return; + +nc->glSetCurrentContextPPAPI(0); +nc->glTerminatePPAPI(); + delete nc; } @@ -51,6 +61,70 @@ nacl_container_ctor() return nc; } +static bool +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) +{ +if (!nc) +return false; Is it really possible for nc to be null? Should this be an assertion instead? If nc can be null here, what sequence of calls leads to that state? Excuse my questions. I don't fully understand the NaCl backend yet. No, this was just a paranoid check, I think this can be removed. If container creation fails (during platform creation) then we will never really end up here. +// There is no way currently to pass a pp::Instance for Waffle, so +// we fetch a map of all instances and if there's only one we select +// that one, otherwise we fail. +const pp::Module::InstanceMap instances = +pp::Module::Get()->current_instances(); + +if (instances.size() != 1) { +wcore_errorf(WAFFLE_ERROR_FATAL, +"Could not find a pp::Instance for Waffle to use.\n"); ^^^ Don't add the newline to error messages. The wcore_error functions add the newline for you. ok, will fix +return false; +} + +pp::Instance *pp_instance = instances.begin()->second; +nc->ctx = pp::Graphics3D(pp_instance, pp::Graphics3D(), cfg->attribs); + +// We need to fetch NaCl specific init, makecurrent and terminate +// functions that communicate with the browser interface. As nacl_config +// currently supports only ES2, this is hardcoded for ES2. +void *glapi = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY); +if (!glapi) { +wcore_errorf(WAFFLE_ERROR_FATAL, "dlopen failed: %s", dlerror()); +return false; +} + +#define RESOLVE(func) \ +nc->func = (typeof(nc->func)) dlsym(glapi, (#func)); \ +if (!nc->func) { \ +wcore_errorf(WAFFLE_ERROR_FATAL, "failed to resolve %s", #func); \ +return false; \ +} + +RESOLVE(glInitializePPAPI); +RESOLVE(glSetCurrentContextPPAPI); +RESOLVE(glTerminatePPAPI); + +#undef RESOLVE + +dlclose(glapi); + +if (!nc->glInitializePPAPI(pp::Module::Get()->get_browser_interface())) { +wcore_errorf(WAFFLE_ERROR_FATAL, +"Unable to initialize GL PPAPI!\n"); +return false; +} + +if (nc->ctx.is_null()) +return false; + +if (!pp_instance->BindGraphics(nc->ctx)) { +wcore_errorf(WAFFLE_ERROR_FATAL, "Unable to bind 3D context.\n"); The term "3D context" is a little to vague. Please mention NaCl, Pepper, or something similar in the error message. Maybe call it a "NaCl 3D Context", for example. yes, I'll try to improve error messaging in geenral, I just noticed I'm not catching possible errors on swapbuffers, I will be adding that. +nc->ctx = pp::Graphics3D(); +nc->glSetCurrentContextPPAPI(0); +return false; +} + +return true; +} + }; // namespace waffle ends extern "C" struct nacl_container* @@ -64,3 +138,10 @@ nacl_teardown(nacl_container *nc) { waffle::nacl_container_dtor(reinterpret_cast(nc)); } + +extern "C" bool +nacl_context_init(struct nacl_container *nc, struct nacl_config *cfg) +{ +return waffle::nacl_context_init( + reinterpret_cast(nc), cfg); +} diff --git a/src/waffle/nacl/nacl_container.h b/src/waffle/nacl/nacl_container.h index 61d935c..81472cc 100644 --- a/src/waffle/nacl/nacl_container.h +++ b/src/waffle/nacl/nacl_container.h @@ -25,13 +25,21 @@ #ifdef __cplusplus +#include + extern "C" { #endif +#include "nacl_config.h" +#include "wcore_error.h" + +#define NACL_GLES2_LIBRARY "libppapi_gles2.so" + struct nacl_container;
Re: [waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
On 01/22/2015 11:59 PM, Tapani Pälli wrote: > Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. > > Signed-off-by: Tapani Pälli > --- > src/waffle/nacl/nacl_container.cpp | 81 > ++ > src/waffle/nacl/nacl_container.h | 8 > src/waffle/nacl/nacl_context.c | 6 +++ > 3 files changed, 95 insertions(+) > > diff --git a/src/waffle/nacl/nacl_container.cpp > b/src/waffle/nacl/nacl_container.cpp > index 3e8073c..e352da9 100644 > --- a/src/waffle/nacl/nacl_container.cpp > +++ b/src/waffle/nacl/nacl_container.cpp > @@ -24,12 +24,18 @@ > // 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/cpp/module.h" > #include "nacl_container.h" > > namespace waffle { > > struct nacl_container { > pp::Graphics3D ctx; > + > +bool (*glInitializePPAPI) (PPB_GetInterface); > +void (*glSetCurrentContextPPAPI) (PP_Resource); > +bool (*glTerminatePPAPI) (void); > }; > > static void > @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) > { > if (!nc) > return; > + > +nc->glSetCurrentContextPPAPI(0); > +nc->glTerminatePPAPI(); > + > delete nc; > } > > @@ -51,6 +61,70 @@ nacl_container_ctor() > return nc; > } > > +static bool > +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) > +{ > +if (!nc) > +return false; Is it really possible for nc to be null? Should this be an assertion instead? If nc can be null here, what sequence of calls leads to that state? Excuse my questions. I don't fully understand the NaCl backend yet. > +// There is no way currently to pass a pp::Instance for Waffle, so > +// we fetch a map of all instances and if there's only one we select > +// that one, otherwise we fail. > +const pp::Module::InstanceMap instances = > +pp::Module::Get()->current_instances(); > + > +if (instances.size() != 1) { > +wcore_errorf(WAFFLE_ERROR_FATAL, > +"Could not find a pp::Instance for Waffle to use.\n"); ^^^ Don't add the newline to error messages. The wcore_error functions add the newline for you. > +return false; > +} > + > +pp::Instance *pp_instance = instances.begin()->second; > +nc->ctx = pp::Graphics3D(pp_instance, pp::Graphics3D(), cfg->attribs); > + > +// We need to fetch NaCl specific init, makecurrent and terminate > +// functions that communicate with the browser interface. As nacl_config > +// currently supports only ES2, this is hardcoded for ES2. > +void *glapi = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY); > +if (!glapi) { > +wcore_errorf(WAFFLE_ERROR_FATAL, "dlopen failed: %s", dlerror()); > +return false; > +} > + > +#define RESOLVE(func) \ > +nc->func = (typeof(nc->func)) dlsym(glapi, (#func)); \ > +if (!nc->func) { \ > +wcore_errorf(WAFFLE_ERROR_FATAL, "failed to resolve %s", #func); \ > +return false; \ > +} > + > +RESOLVE(glInitializePPAPI); > +RESOLVE(glSetCurrentContextPPAPI); > +RESOLVE(glTerminatePPAPI); > + > +#undef RESOLVE > + > +dlclose(glapi); > + > +if (!nc->glInitializePPAPI(pp::Module::Get()->get_browser_interface())) { > +wcore_errorf(WAFFLE_ERROR_FATAL, > +"Unable to initialize GL PPAPI!\n"); > +return false; > +} > + > +if (nc->ctx.is_null()) > +return false; > + > +if (!pp_instance->BindGraphics(nc->ctx)) { > +wcore_errorf(WAFFLE_ERROR_FATAL, "Unable to bind 3D context.\n"); The term "3D context" is a little to vague. Please mention NaCl, Pepper, or something similar in the error message. Maybe call it a "NaCl 3D Context", for example. > +nc->ctx = pp::Graphics3D(); > +nc->glSetCurrentContextPPAPI(0); > +return false; > +} > + > +return true; > +} > + > }; // namespace waffle ends > > extern "C" struct nacl_container* > @@ -64,3 +138,10 @@ nacl_teardown(nacl_container *nc) > { > > waffle::nacl_container_dtor(reinterpret_cast(nc)); > } > + > +extern "C" bool > +nacl_context_init(struct nacl_container *nc, struct nacl_config *cfg) > +{ > +return waffle::nacl_context_init( > + reinterpret_cast(nc), cfg); > +} > diff --git a/src/waffle/nacl/nacl_container.h > b/src/waffle/nacl/nacl_container.h > index 61d935c..81472cc 100644 > --- a/src/waffle/nacl/nacl_container.h > +++ b/src/waffle/nacl/nacl_container.h > @@ -25,13 +25,21 @@ > > #ifdef __cplusplus > > +#include > + > extern "C" { > #endif > > +#include "nacl_config.h" > +#include "wcore_error.h" > + > +#define NACL_GLES2_LIBRARY "libppapi_gles2.so" > + > struct nacl_container; > > struct nacl_container *nacl_init(); > void nacl_teardown(struct nacl_container *nc); > +bool nac
[waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create
Patch creates and initializes pp::Graphics3D context for OpenGL ES 2.0. Signed-off-by: Tapani Pälli --- src/waffle/nacl/nacl_container.cpp | 81 ++ src/waffle/nacl/nacl_container.h | 8 src/waffle/nacl/nacl_context.c | 6 +++ 3 files changed, 95 insertions(+) diff --git a/src/waffle/nacl/nacl_container.cpp b/src/waffle/nacl/nacl_container.cpp index 3e8073c..e352da9 100644 --- a/src/waffle/nacl/nacl_container.cpp +++ b/src/waffle/nacl/nacl_container.cpp @@ -24,12 +24,18 @@ // 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/cpp/module.h" #include "nacl_container.h" namespace waffle { struct nacl_container { pp::Graphics3D ctx; + +bool (*glInitializePPAPI) (PPB_GetInterface); +void (*glSetCurrentContextPPAPI) (PP_Resource); +bool (*glTerminatePPAPI) (void); }; static void @@ -37,6 +43,10 @@ nacl_container_dtor(waffle::nacl_container *nc) { if (!nc) return; + +nc->glSetCurrentContextPPAPI(0); +nc->glTerminatePPAPI(); + delete nc; } @@ -51,6 +61,70 @@ nacl_container_ctor() return nc; } +static bool +nacl_context_init(waffle::nacl_container *nc, struct nacl_config *cfg) +{ +if (!nc) +return false; + +// There is no way currently to pass a pp::Instance for Waffle, so +// we fetch a map of all instances and if there's only one we select +// that one, otherwise we fail. +const pp::Module::InstanceMap instances = +pp::Module::Get()->current_instances(); + +if (instances.size() != 1) { +wcore_errorf(WAFFLE_ERROR_FATAL, +"Could not find a pp::Instance for Waffle to use.\n"); +return false; +} + +pp::Instance *pp_instance = instances.begin()->second; +nc->ctx = pp::Graphics3D(pp_instance, pp::Graphics3D(), cfg->attribs); + +// We need to fetch NaCl specific init, makecurrent and terminate +// functions that communicate with the browser interface. As nacl_config +// currently supports only ES2, this is hardcoded for ES2. +void *glapi = dlopen(NACL_GLES2_LIBRARY, RTLD_LAZY); +if (!glapi) { +wcore_errorf(WAFFLE_ERROR_FATAL, "dlopen failed: %s", dlerror()); +return false; +} + +#define RESOLVE(func) \ +nc->func = (typeof(nc->func)) dlsym(glapi, (#func)); \ +if (!nc->func) { \ +wcore_errorf(WAFFLE_ERROR_FATAL, "failed to resolve %s", #func); \ +return false; \ +} + +RESOLVE(glInitializePPAPI); +RESOLVE(glSetCurrentContextPPAPI); +RESOLVE(glTerminatePPAPI); + +#undef RESOLVE + +dlclose(glapi); + +if (!nc->glInitializePPAPI(pp::Module::Get()->get_browser_interface())) { +wcore_errorf(WAFFLE_ERROR_FATAL, +"Unable to initialize GL PPAPI!\n"); +return false; +} + +if (nc->ctx.is_null()) +return false; + +if (!pp_instance->BindGraphics(nc->ctx)) { +wcore_errorf(WAFFLE_ERROR_FATAL, "Unable to bind 3D context.\n"); +nc->ctx = pp::Graphics3D(); +nc->glSetCurrentContextPPAPI(0); +return false; +} + +return true; +} + }; // namespace waffle ends extern "C" struct nacl_container* @@ -64,3 +138,10 @@ nacl_teardown(nacl_container *nc) { waffle::nacl_container_dtor(reinterpret_cast(nc)); } + +extern "C" bool +nacl_context_init(struct nacl_container *nc, struct nacl_config *cfg) +{ +return waffle::nacl_context_init( + reinterpret_cast(nc), cfg); +} diff --git a/src/waffle/nacl/nacl_container.h b/src/waffle/nacl/nacl_container.h index 61d935c..81472cc 100644 --- a/src/waffle/nacl/nacl_container.h +++ b/src/waffle/nacl/nacl_container.h @@ -25,13 +25,21 @@ #ifdef __cplusplus +#include + extern "C" { #endif +#include "nacl_config.h" +#include "wcore_error.h" + +#define NACL_GLES2_LIBRARY "libppapi_gles2.so" + struct nacl_container; struct nacl_container *nacl_init(); void nacl_teardown(struct nacl_container *nc); +bool nacl_context_init(struct nacl_container *nc, struct nacl_config *cfg); #ifdef __cplusplus }; diff --git a/src/waffle/nacl/nacl_context.c b/src/waffle/nacl/nacl_context.c index 2e68a64..e8adeb0 100644 --- a/src/waffle/nacl/nacl_context.c +++ b/src/waffle/nacl/nacl_context.c @@ -47,12 +47,18 @@ nacl_context_create(struct wcore_platform *wc_plat, struct wcore_context *wc_share_ctx) { struct nacl_context *self; +struct nacl_config *config = nacl_config(wc_config); +struct nacl_platform *platform = nacl_platform(wc_plat); bool ok = true; self = wcore_calloc(sizeof(*self)); if (self == NULL) return NULL; +ok = nacl_context_init(platform->nacl, config); +if (!ok) +goto error; + ok = wcore_context_init(&self->wcore, wc_config); if (!ok) goto error; -- 2.1.0 _