flypig.co.uk

As we left things yesterday we were looking at nsWindow::GetNativeData() and the fact it does things the same way for both ESR 78 and ESR 91, simply returning GetGLContext() in response to a NS_NATIVE_OPENGL_CONTEXT parameter being passed in.

Interestingly, this call to GetGLContext() is exactly where we saw the GLContext being created on ESR 78 on day 223. So it's quite possible that both calls are going through on both versions, yet while it succeeds on ESR 78, it fails on ESR 91. It's not at all clear why that might be. Time to find out.

Crucially there is a difference between the code in GetGLContext() on ESR 78 compared to the code on ESR 91. Here's the very start of the ESR 78 method:

GLContext*
nsWindow::GetGLContext() const
{
  LOGT("this:%p, UseExternalContext:%d", this, sUseExternalGLContext);
  if (sUseExternalGLContext) {
    void* context = nullptr;
    void* surface = nullptr;
    void* display = nullptr;
[...]

Notice how the entrance to the main body of the function is gated by a variable called sUseExternalGLContext. In order for this method to return something non-null, it's essential that this is set to true. On ESR 91 this has changed from a variable to a static preference that looks like this:

GLContext*
nsWindow::GetGLContext() const
{
  LOGT("this:%p, UseExternalContext:%d", this,
      StaticPrefs::embedlite_compositor_external_gl_context());
  if (StaticPrefs::embedlite_compositor_external_gl_context()) {
    void* context = nullptr;
    void* surface = nullptr;
    void* display = nullptr;
[...]

This was actually a change I made myself and it's not really a very dramatic change at all. In ESR 78 the sUseExternalGLContext variable was being mirrored from a static preference, which is one that can be read very quickly, so there was no real reason to copy it into a variable. Hence I just switched out the variable with direct accesses of the static pref instead. That was all documented back on Day 97.

The value of the static pref is set to true by default, as we can see in the StaticPrefList.yaml file:

# Make gecko compositor use GL context/surface provided by the application
-   name: embedlite.compositor.external_gl_context
    type: bool
    value: true
    mirror: always

However this value can be overidden by the preferences, stored in ~/.local/share/org.sailfishos/browser/.mozilla/prefs.js. Looking inside that file I see the following:

user_pref("embedlite.compositor.external_gl_context", false);

I've switched that back to true and now, when I run the browser... woohoo! Rendering works again. Great! This is a huge relief. The onscreen rendering pipeline is still working just fine.

However, the next time I fire the browser up it crashes again. And when I check the preferences file I can see the value has switched back to false. That's because I've set it to do that in WebEngineSettings that's part of sailfish-components-webview:

    // Ensure the renderer is configured correctly
    engineSettings->setPreference(QStringLiteral(
        "gfx.webrender.force-disabled"),
        QVariant(true));
    engineSettings->setPreference(QStringLiteral(
        "embedlite.compositor.external_gl_context"),
        QVariant(false));

And the reason for that was documented back on Day 165, where I wrote this:
 
But I obviously wasn't entirely comfortable with this at the time and went on to write the following:

I'm going to set it to <tt>false</tt> explicitly for the WebView. But this 
    immediately makes me feel nervous: this setting isn't new and there's a 
    reason it's not being touched in the WebView code. It makes me think that 
    I'm travelling down a rendering pipeline path that I shouldn't be.

And so it panned out. Looming back at what I wrote then, the issue I was trying to address by flipping the static preference was the need to enter the condition in the following bit of code:

PLayerTransactionParent*
EmbedLiteCompositorBridgeParent::AllocPLayerTransactionParent(const 
    nsTArray<LayersBackend>& aBackendHints,
    const LayersId& 
    aId)
{
  PLayerTransactionParent* p =
    CompositorBridgeParent::AllocPLayerTransactionParent(aBackendHints, aId);

  EmbedLiteWindowParent *parentWindow = EmbedLiteWindowParent::From(mWindowId);
  if (parentWindow) {
    parentWindow->GetListener()->CompositorCreated();
  }

  if (!StaticPrefs::embedlite_compositor_external_gl_context()) {
    // Prepare Offscreen rendering context
    PrepareOffscreen();
  }
  return p;
}

I wanted PrepareOffscreen() to be called and negating this preference seemed the neatest and easiest way to make it happen.

But now I need the opposite, so for the time being I've disabled the forcing of this preference in sailfish-components-webview. That change necessitated (of course) a rebuild and reinstallation of the component.

The browser is now back up and running. Hooray! But the WebView now crashes before it even attempts to render a page. That's not unexpected and presumably will take us back to this failure to call PrepareOffscreen(). I'm going to investigate that further, but not today, it'll have to wait until tomorrow.

Even though this was one step backwards, one step forwards, it still feels like progress. We'll get there.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.