flypig.co.uk

I'm back at things feeling a lot fresher today after my head ended up swimming last night.

This new clarity allowed me to dig through the code a bit more and I'm a lot clearer about how things are supposed to fit together as a result.

In ESR 91 the key entry point method where the graphics initialisation from qtmozembed meets the gecko library is called GLContextProviderEGL::CreateWrappingExisting(). This should accept an aDisplay parameter that's created externally and passed in by qtmozembed.

Currently the code then goes off and performs some minimal initialisation by calling EglDisplay::Create(). But there are two versions of this Create() method. The method I really want called should then go on to call GLLibraryEGL::Init(), but instead we have it inserting the display into a list and returning. I do want the display in that list, but I don't want it to skip all the other initialisation steps.

The Create() method creates a copy of the library and uses it to call Init(). But we actually already have our library because we created it in GLContextProviderEGL::CreateWrappingExisting(). So we can short-circuit the call to Create() and call GLLibraryEGL::Init() directly instead.

So that's our first requirement. From that point on we're no longer in a static method, our lib becomes our this and we don't need to keep track of it anymore.

However there's another critical change that happened from ESR 78 to ESR 91. The GLLibraryEGL::DoEnsureInitialized method in ESR 78 (which has been renamed to this Init() we were talking about in ESR 91) calls GLLibraryEGL::CreateDisplay(). This looks like a key method, and it's also the method that I've noticed isn't getting called in ESR 91 (it was this realisation that confirmed my suspicions that this part of the code is broken).

So we're going to have to add it back in to the ESR 91 code. It looks like if we do this, the code will end up calling our Create() method (the same one we're calling now) but with a lot of other initialisation happening as well.

That should lead us to a better place than we are now.

So the steps are:

1. Have GLContextProviderEGL::CreateWrappingExisting() call GLLibraryEGL::Init() rather than EglDisplay::Create() as it is now.
2. Adjust GLLibraryEGL::Init() so that it calls GLLibraryEGL::CreateDisplay(). We can use the ESR 78 code as a guide for this.
3. The call to GLLibraryEGL::CreateDisplay() results in a call to a static GetAndInitDisplay() method, which needs to consume an aDisplay parameter.
4. Check that the call to GetAndInitDisplay() will eventually result in a call to the correct version of EglDisplay::Create().

In ESR 78 the GLLibraryEGL::CreateDisplay() method was returning an instance of EGLDisplay. Now it's returning an instance of EglDisplay (note the capitalisation). I think that's okay: the latter is essentially a wrapper for holding multiple of the former.

But it means we'll have to get our updated version of GLLibraryEGL::Init() to return the EglDisplay instance rather than the Boolean it's returning now. In ESR 78 the value was stored in the mEGLDisplay class member, but this has been removed. We need to return it instead.

It's not clear to me that this is the only problem to fix, but this definitely looks broken as the code stands right now. Fixing it can't do any harm.

It's an early start for me tomorrow so too late for me to make the changes tonight. It'll be my task for tomorrow.

For all the other entries in my developer diary, check out the Gecko-dev Diary page.