flypig.co.uk

Good news this morning: all of the changes I made yesterday have gone through except one. The outlier is the reintroduction of the CreateWrappingExisting() method. While adding this fixed the dangling function call we saw yesterday, it's brought in a few new errors related to code that's inside the function. Broken code added by me without knowing what it was doing? Not such a surprise really.

86:46.60 gfx/gl
187:04.10 In file included from Unified_cpp_gfx_gl0.cpp:47:
187:04.10 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp: In static member
          function ‘static already_AddRefed mozilla::
          gl::GLContextProviderEGL::CreateWrappingExisting(void*, void*, void*)’:
187:04.10 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1001:22: error:
          ‘EnsureInitialized’ is not a member of ‘mozilla::gl::GLLibraryEGL’
187:04.10    if (!GLLibraryEGL::EnsureInitialized(false, &discardFailureId, aDisplay)) {
187:04.10                       ^~~~~~~~~~~~~~~~~
187:04.10 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1008:35: error:
          ‘Get’ is not a member of ‘mozilla::gl::GLLibraryEGL’
187:04.10    const auto& egl = GLLibraryEGL::Get();
187:04.10                                    ^~~
187:04.10 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1009:3: error:
          ‘SurfaceCaps’ was not declared in this scope
187:04.10    SurfaceCaps caps = SurfaceCaps::Any();
187:04.10    ^~~~~~~~~~~
187:04.12 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1009:3: note:
          suggested alternative: ‘aSurface’
187:04.12    SurfaceCaps caps = SurfaceCaps::Any();
187:04.12    ^~~~~~~~~~~
187:04.12    aSurface
187:04.12 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1012:55: error:
          ‘caps’ was not declared in this scope
187:04.12        new GLContextEGL(egl, CreateContextFlags::NONE, caps, false, config,
187:04.12                                                        ^~~~
187:04.13 ${PROJECT}/gecko-dev/gfx/gl/GLContextProviderEGL.cpp:1012:55: note:
          suggested alternative: ‘css’
187:04.13        new GLContextEGL(egl, CreateContextFlags::NONE, caps, false, config,
187:04.13                                                        ^~~~
187:04.13                                                        css
187:06.71 In file included from Unified_cpp_gfx_gl0.cpp:137:
187:06.71 ${PROJECT}/gecko-dev/gfx/gl/SharedSurface.cpp: In static member
          function ‘static mozilla::UniquePtr
          mozilla::gl::SurfaceFactory::Create(mozilla::gl::GLContext*,
          mozilla::layers::TextureType)’:
187:06.71 ${PROJECT}/gecko-dev/gfx/gl/SharedSurface.cpp:84:9: warning:
          unused variable ‘gl’ [-Wunused-variable]
187:06.71    auto& gl = *pGl;
187:06.71          ^~
187:10.79 make[4]: *** [${PROJECT}/gecko-dev/config/rules.mk:676: Unified_cpp_gfx_gl0.o] Error 1

By the looks of the errors there have been other changes to GLLibraryEGL that have left us hanging, as well as the removal (or maybe refactoring) of SurfaceCaps. Both concrete leads for us to look into.

It occurs to me that these errors may be masking errors with some of the other changes we made after this change, but we won't know that until after a rebuild.

So, how to fix things? The problem method here is GLContextProviderEGL::CreateWrappingExisting(). The purpose of GLContextProviderEGL is essentially as a route into management of the graphics context. This could be using OpenGL, Vulkan, Direct3D, or in our case EGL: the "Khronos Native Platform Graphics Interface" (possibly EGL once stood for the "Embedded-System Graphics Library").

The Gecko approach to EGL is to use a dynamic library interface, with graphics rendering to an EGLDisplay, which is an integer that references where to output graphics to; the actual meaning of the number is opaque and internal to EGL. In ESR 78 there was essentially only one EGLDisplay used for all the rendering. But since then the code has switched to allowing the use of multiple EGLDisplay instances. According to what I've read in the source code and commit messages, this is important for things like WebGL. The changes to the code seem to be to allow these multiple instances to be handled properly.

More specifically, rather than using the EGLDisplay internally, there is now a wrapper EglDisplay class (note the slightly different capitalisation) which wraps the EGLDisplay. In addition, all these EglDisplay wrapper instances are stored in a map so that the correct instance can be associated with the correct display.

All great, except on Sailfish OS the creation and management of the EGLDisplay is handled by QtMozEmbed, not Gecko. And that's where this CreateWrappingExisting() method comes in. The upper layers of the Sailfish Browser call into EmbedLite with an already created EGLDisplay instance (note the capitalisation), which gets passed to CreateWrappingExisting() so that this instance can be used rather than creating a new one.

That path has now gone. I tried to put it back, but the Gecko code isn't set up to handle EGLDisplay instances passed in from elsewhere anymore, so all of the support code needed for this has gone with it.

In addition, the way to handle the EglLibrary, which is the interface used to the graphics library Gecko uses, has changed too.

By digging through the support code I was able to find a method that will return the library if it exists, or set it up otherwise. That gets us one step in the right direction.

Even more crucially, there is also this EGLDisplay::Create() method which accepts an EGLDisplay instance and wraps it in an EglDisplay object (again, note the capitalisation). This is the crucial missing piece of the puzzle that I'm after. Having found this I've been able to piece back together all of the missing functionality needed by CreateWrappingExisting(). Or at least, that's what I think I've done!

As always I'm developing in the dark here. I've made changes that won't be testable until after everything finally builds. I'm not expecting to have come up with working code, but I am hoping that the time spent on these changes now will mean less effort once the code can be executed.

That's more than enough for today though. I've made the changes and set the build running.

Ah, that's not quite all though. I want to briefly mention some other relevant Gecko progress as well.

This morning we had the Sailfish OS Community Meeting (for those of you comparing diaries, you'll notice I'm still a little behind reality with these dev diaries).

During the meeting I took the opportunity to raise a number of Gecko related questions. I asked about the possibility for the Linux headers to be updated, something which we covered on Day 10. Gecko needs version 4.3 or newer of the headers to build properly. I was pleased to hear that Jolla are thinking about whether this would be possible.

In addition we discussed some of the Pull Requests to other packages needed for the Gecko build. The ICU 70.1 and cbindgen 0.19.0 updates have now been merged, which is great news. An interesting aside mentioned by Raine during the meeting: Until now it's been a mystery why Gecko was refusing to build for this target. So now we know!

There was also progress on direc85's gcc patch as well. Both direc85 and Jolla have put a lot of work in to all these PRs and as of this morning the gcc changes have built for aarch64 on OBS. This means I can finally test out whether the patch actually fixes the underlying issue or not.

Unfortunately I didn't have time to perform this check today, but this will be on my to-do list for tomorrow.

That really is it for today. As always, if you want to read my other posts they're available in my full Gecko Dev Diary.