flypig.co.uk

It felt like I had to stop halfway through a thought yesterday. But sometimes tiredness gets the better of me, I can feel myself spiralling into incoherence and the only sensible thing to do is head to bed. Sometimes it's a slow descent while relaxing or reading a book; other times I reach the incoherent end of the spectrum before my mind has even caught up with the fact I'm heading there.

So let me try to regroup. What we learnt yesterday was that OffscreenSize() previously returned mScreen->Size() and mScreen was created in InitOffscreen(). This InitOffscreen() method no longer exists — it was removed in D75055 — but was originally called in GLContextProviderEGL::CreateOffscreen().

The GLContextProviderEGL::CreateOffscreen() method also no longer exists in the codebase, replaced as it was in D79390:

$ git log -1 -S "GLContextProviderEGL::CreateOffscreen" \
    gecko-dev/gfx/gl/GLContextProviderEGL.cpp
commit 4232c2c466220d42223443bd5bd2f3c849123380
Author: Jeff Gilbert <jgilbert@mozilla.com>
Date:   Mon Jun 15 18:26:12 2020 +0000

    Bug 1632249 - Replace GLContextProvider::CreateOffscreen with
    GLContext::CreateOffscreenDefaultFb. r=lsalzman
    
    Differential Revision: https://phabricator.services.mozilla.com/D79390

Looking at the ESR 91 code and the diffs applied to them it's not immediately obvious to me where this was getting called from and what's replacing it now, but we can get a callstack for how that was being called using the debugger on ESR 78. Here's the (abridged) backtrace:

(gdb) delete break
Delete all breakpoints? (y or n) y
(gdb) b GLContextProviderEGL::CreateOffscreen Breakpoint 6 at 0x7fb8e839f0:
    file gfx/gl/GLContextProviderEGL.cpp, line 1400.
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /usr/bin/harbour-webview 
[...]
Thread 36 "Compositor" hit Breakpoint 6, mozilla::gl::GLContextProviderEGL::
    CreateOffscreen (size=..., minCaps=..., 
    flags=flags@entry=mozilla::gl::CreateContextFlags::REQUIRE_COMPAT_PROFILE,
    out_failureId=out_failureId@entry=0x7fa516f378)
    at gfx/gl/GLContextProviderEGL.cpp:1400
1400        CreateContextFlags flags, nsACString* const out_failureId) {
(gdb) bt
#0  mozilla::gl::GLContextProviderEGL::CreateOffscreen (size=..., minCaps=..., flags=flags@entry=mozilla::gl::CreateContextFlags::REQUIRE_COMPAT_PROFILE, 
    out_failureId=out_failureId@entry=0x7fa516f378)
    at gfx/gl/GLContextProviderEGL.cpp:1400
#1  0x0000007fb8ee475c in mozilla::layers::CompositorOGL::CreateContext
    (this=0x7eac003420)
    at gfx/layers/opengl/CompositorOGL.cpp:250
#2  mozilla::layers::CompositorOGL::CreateContext (this=0x7eac003420)
    at gfx/layers/opengl/CompositorOGL.cpp:223
#3  0x0000007fb8f053bc in mozilla::layers::CompositorOGL::Initialize
    (this=0x7eac003420, out_failureReason=0x7fa516f730)
    at gfx/layers/opengl/CompositorOGL.cpp:374
#4  0x0000007fb8fdcf7c in mozilla::layers::CompositorBridgeParent::NewCompositor
    (this=this@entry=0x7f8c99dc60, aBackendHints=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1534
#5  0x0000007fb8fe65e8 in mozilla::layers::CompositorBridgeParent::
    InitializeLayerManager (this=this@entry=0x7f8c99dc60, aBackendHints=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1491
#6  0x0000007fb8fe6730 in mozilla::layers::CompositorBridgeParent::
    AllocPLayerTransactionParent (this=this@entry=0x7f8c99dc60,
    aBackendHints=..., aId=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1587
#7  0x0000007fbb2e31b4 in mozilla::embedlite::EmbedLiteCompositorBridgeParent::
    AllocPLayerTransactionParent (this=0x7f8c99dc60, aBackendHints=..., 
    aId=...) at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.cpp:77
#8  0x0000007fb88c13d0 in mozilla::layers::PCompositorBridgeParent::
    OnMessageReceived (this=0x7f8c99dc60, msg__=...)
    at PCompositorBridgeParent.cpp:1391
#9  0x0000007fb88f86ac in mozilla::layers::PCompositorManagerParent::
    OnMessageReceived (this=<optimized out>, msg__=...)
    at obj-build-mer-qt-xr/dist/include/mozilla/ipc/ProtocolUtils.h:866
[...]
#24 0x0000007fbe70d89c in ?? () from /lib64/libc.so.6
(gdb) 

So examining the code at frame 1, we see that what was a call to CreateOffscreen():

  // Allow to create offscreen GL context for main Layer Manager
  if (!context && gfxEnv::LayersPreferOffscreen()) {
    SurfaceCaps caps = SurfaceCaps::ForRGB();
    caps.preserve = false;
    caps.bpp16 = gfxVars::OffscreenFormat() == SurfaceFormat::R5G6B5_UINT16;

    nsCString discardFailureId;
    context = GLContextProvider::CreateOffscreen(
        mSurfaceSize, caps, CreateContextFlags::REQUIRE_COMPAT_PROFILE,
        &discardFailureId);
  }

Is now a call to CreateHeadless():

  // Allow to create offscreen GL context for main Layer Manager
  if (!context && gfxEnv::LayersPreferOffscreen()) {
    nsCString discardFailureId;
    context = GLContextProvider::CreateHeadless(
        {CreateContextFlags::REQUIRE_COMPAT_PROFILE}, &discardFailureId);
    if (!context->CreateOffscreenDefaultFb(mSurfaceSize)) {
      context = nullptr;
    }
  }

Whether we're on ESR 78 or ESR 91, this is all happening inside CompositorOGL::CreateContext().

Looking at the difference between the previous code that was called in CreateOffscreen() and the new code being called in CreateHeadless() my heart sinks a bit. There's so much that's been removed. It's true that CreateOffscreen() did go on to call CreateHeadless() in ESR 78, but there's so much other initialisation code in ESR 78, I just can't believe we can safely throw it all away.

But I'm going to persevere down this road I've started on, gradually building things back up only where they're needed to get things working. Right now that still means fixing the crash when OffscreenSize() is called.

I've not quite reached the point where the two sides of this circle meet up and the correct position to create the mBackBuffer emerges, but I feel like this exploration is getting us closer.

It's time for work now, I'll pick this up later on today.

[...]

After thinking on this some more, I've come to the conclusion that the right place to set up the mBackBuffer variable is in, or near, the call to GLContextProviderEGL::CreateHeadless(). It's there that the mScreen object would have been created in ESR 78 and checking with the debugger shows that the ordering is appropriate: CreateHeadless() gets called before CompositeToDefaultTarget(), which is what we need.

(gdb) delete break
(gdb) b EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget
Breakpoint 3 at 0x7ff3667880: EmbedLiteCompositorBridgeParent::
    CompositeToDefaultTarget. (2 locations)
(gdb) b GLContextProviderEGL::CreateHeadless
Breakpoint 4 at 0x7ff1133740: file gfx/gl/GLContextProviderEGL.cpp, line 1245.
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /usr/bin/harbour-webview 
[...]
Thread 36 "Compositor" hit Breakpoint 4, mozilla::gl::GLContextProviderEGL::
    CreateHeadless (desc=..., out_failureId=out_failureId@entry=0x7f1faad1c8)
    at gfx/gl/GLContextProviderEGL.cpp:1245
1245        const GLContextCreateDesc& desc, nsACString* const out_failureId) {
(gdb) bt
#0  mozilla::gl::GLContextProviderEGL::CreateHeadless (desc=...,
    out_failureId=out_failureId@entry=0x7f1faad1c8)
    at gfx/gl/GLContextProviderEGL.cpp:1245
#1  0x0000007ff119b81c in mozilla::layers::CompositorOGL::CreateContext
    (this=this@entry=0x7ee0002f50)
    at gfx/layers/opengl/CompositorOGL.cpp:250
#2  0x0000007ff11b0e24 in mozilla::layers::CompositorOGL::Initialize
    (this=0x7ee0002f50, out_failureReason=0x7f1faad520)
    at gfx/layers/opengl/CompositorOGL.cpp:389
#3  0x0000007ff12c6864 in mozilla::layers::CompositorBridgeParent::
    NewCompositor (this=this@entry=0x7fc4b3d260, aBackendHints=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1493
#4  0x0000007ff12d18e0 in mozilla::layers::CompositorBridgeParent::
    InitializeLayerManager (this=this@entry=0x7fc4b3d260, aBackendHints=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1436
#5  0x0000007ff12d1a10 in mozilla::layers::CompositorBridgeParent::
    AllocPLayerTransactionParent (this=this@entry=0x7fc4b3d260,
    aBackendHints=..., aId=...)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:1546
#6  0x0000007ff3668238 in mozilla::embedlite::EmbedLiteCompositorBridgeParent::
    AllocPLayerTransactionParent (this=0x7fc4b3d260, aBackendHints=..., aId=...)
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.cpp:80
#7  0x0000007ff0c65ad0 in mozilla::layers::PCompositorBridgeParent::
    OnMessageReceived (this=0x7fc4b3d260, msg__=...)
    at PCompositorBridgeParent.cpp:1285
#8  0x0000007ff0ca9fe4 in mozilla::layers::PCompositorManagerParent::
    OnMessageReceived (this=<optimized out>, msg__=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/ipc/
    ProtocolUtils.h:675
[...]
#22 0x0000007ff6a0489c in ?? () from /lib64/libc.so.6
(gdb) c
Continuing.
[New LWP 2694]
=============== Preparing offscreen rendering context ===============
[New LWP 2695]

Thread 36 "Compositor" hit Breakpoint 3, non-virtual thunk to mozilla::embedlite::EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget
    (mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>) ()
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.h:58
58        virtual void CompositeToDefaultTarget(VsyncId aId) override;
(gdb) bt
#0  non-virtual thunk to mozilla::embedlite::EmbedLiteCompositorBridgeParent::
    CompositeToDefaultTarget(mozilla::layers::BaseTransactionId
    <mozilla::VsyncIdType>) ()
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.h:58
#1  0x0000007ff12b808c in mozilla::layers::CompositorVsyncScheduler::
    ForceComposeToTarget (this=0x7fc4d0df60, aTarget=aTarget@entry=0x0, 
    aRect=aRect@entry=0x0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/layers/
    LayersTypes.h:82
#2  0x0000007ff12b80e8 in mozilla::layers::CompositorBridgeParent::
    ResumeComposition (this=this@entry=0x7fc4b3d260)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:313
#3  0x0000007ff12b8174 in mozilla::layers::CompositorBridgeParent::
    ResumeCompositionAndResize (this=0x7fc4b3d260, x=<optimized out>,
    y=<optimized out>, width=<optimized out>, height=<optimized out>)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:794
#4  0x0000007ff12b0d10 in mozilla::detail::RunnableMethodArguments<int,
    int, int, int>::applyImpl<mozilla::layers::CompositorBridgeParent, void
    (mozilla::layers::CompositorBridgeParent::*)(int, int, int, int),
    StoreCopyPassByConstLRef<int>, StoreCopyPassByConstLRef<int>,
    StoreCopyPassByConstLRef<int>, StoreCopyPassByConstLRef<int>, 0ul, 1ul, 2ul,
    3ul> (args=..., m=<optimized out>, o=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsThreadUtils.h:1151
[...]
#16 0x0000007ff6a0489c in ?? () from /lib64/libc.so.6
(gdb) 

The only problem is that the SwapChain object is held by GLContext whereas CreateHeadless() is part of GLContextProviderEGL(), which has access to very little, let alone the SwapChain. The good news is GLContextProviderEGL does have access to GLContext. The structure is something like this:

class GLContext 
public:
    UniquePtr<SwapChain> mSwapChain;
}

class GLContextEGL final : public GLContext {
}

void EmbedLiteCompositorBridgeParent::PrepareOffscreen() {
    const CompositorBridgeParent::LayerTreeState* state =
    CompositorBridgeParent::GetIndirectShadowTree(RootLayerTreeId());
    GLContext* context = static_cast<CompositorOGL*>(state->mLayerManager->
    GetCompositor())->gl();

    swapChain = new SwapChain();
    new SwapChainPresenter(*swapChain);
    context->mSwapChain.reset(swapChain);
}

already_AddRefed<mozilla::gl::GLContext> CompositorOGL::CreateContext() {
    context = GLContextProvider::CreateHeadless({CreateContextFlags::
    REQUIRE_COMPAT_PROFILE}, &discardFailureId);
    // Here we have access to context->mSwapChain;
}

Based on this, it looks like adding code in to CompositorOGL::CreateContext(), after the call to GLContextProvider::CreateHeadless() (which is actually a call to GLContextProviderEGL::CreateHeadless() might be the right place to put the code to create mBackBuffer.

So, after the calls in CompositorOGL::CreateContext() I've inserted the following code:

  bool success = context->ResizeScreenBuffer(mSurfaceSize);
  if (!success) {
      NS_WARNING("Failed to create SwapChain back buffer");
  }

This will make for my first attempt to fix this. I've set the build running and we'll have to find out for sure whether that's improved the situation tomorrow.

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