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
