[waffle] [PATCH 3/7] nacl: add implementation for waffle_context_create

[Tapani Pälli]

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 <tapani.palli at intel.com>
>> ---
>>   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<waffle::nacl_container*>(nc));
>>   }
>> +
>> +extern "C" bool
>> +nacl_context_init(struct nacl_container *nc, struct nacl_config *cfg)
>> +{
>> +    return waffle::nacl_context_init(
>> +                   reinterpret_cast<waffle::nacl_container*>(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 <dlfcn.h>
>> +
>>   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;
>>
>
> The base object, wcore_context, should be initialize before the
> child object, nacl_context. That's the pattern throughout Waffle,
> and that's how C++ does it too.
>

Yes of course, will fix.

// Tapani