flypig.co.uk

I'm travelling for work at the moment. Last night I stayed at an easyHotel. I'm a fan of the easyHotel concept: clean, cheap and well-located, but also tiny and basic. The fact that having a window is an optional paid-for extra tells you everything you need to know about the chain. There was just enough room around the edge of the bed to squeeze a human leg or two, but certainly no space for a desk. This presented me with a problem when it came to leaving my laptop running. I needed to have the build I started yesterday run overnight. Sending my laptop to sleep wasn't an option.

For a while I contemplated leaving it running on the bed next to me, but I had visions of waking up in the middle of night with my bed on fire or, worse, with my laptop shattered into little pieces having been thrown to the floor.

Eventually the very-helpful hotel staff found me a mini ironing board. After removing the cover it made a passable laptop rack. And so this morning I find myself in a coffee shop in the heart of Birmingham (which is itself the heart of the UK) with a freshly minted set of packages ready to try out on my phone.

The newly installed debug-symbol adorned binaries immediately deliver.

Thread 36 "Compositor" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f39d297d0 (LWP 31614)]
0x0000007ff28a8978 in mozilla::gl::GLScreenBuffer::Size (this=0x0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:290
290     ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h: No 
    such file or directory.
(gdb) bt
#0  0x0000007ff28a8978 in mozilla::gl::GLScreenBuffer::Size (this=0x0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:290
#1  mozilla::gl::GLContext::OffscreenSize (this=this@entry=0x7ee019aa30)
    at gfx/gl/GLContext.cpp:2141
#2  0x0000007ff4e07264 in mozilla::embedlite::EmbedLiteCompositorBridgeParent::
    CompositeToDefaultTarget (this=0x7fc8b77bc0, aId=...)
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.cpp:156
#3  0x0000007ff2a578d8 in mozilla::layers::CompositorVsyncScheduler::
    ForceComposeToTarget (this=0x7fc8ca6a10, aTarget=aTarget@entry=0x0, 
    aRect=aRect@entry=0x0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/layers/LayersTypes.h:
    82
#4  0x0000007ff2a57934 in mozilla::layers::CompositorBridgeParent::
    ResumeComposition (this=this@entry=0x7fc8b77bc0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:313
#5  0x0000007ff2a579c0 in mozilla::layers::CompositorBridgeParent::
    ResumeCompositionAndResize (this=0x7fc8b77bc0, x=<optimized out>, 
    y=<optimized out>, 
    width=<optimized out>, height=<optimized out>)
    at gfx/layers/ipc/CompositorBridgeParent.cpp:794
#6  0x0000007ff2a5055c 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
[...]
#18 0x0000007fefba889c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/
    clone.S:78
(gdb) 

The basis of the problem is that GLContext::OffscreenSize() is calling Size() on a value of GLScreenBuffer mScreen which is set to null:

const gfx::IntSize& GLContext::OffscreenSize() const {
  MOZ_ASSERT(IsOffscreen());
  return mScreen->Size();
}

Checking the diff, this GLContext::OffscreenSize() method was added by me as part of the recent shake-up of the offscreen render code. Here's the change I made in EmbedLiteCompositorBridgeParent.cpp that's making this call:

@@ -151,8 +153,7 @@ EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget(
    VsyncId aId)
 
   if (context->IsOffscreen()) {
     MutexAutoLock lock(mRenderMutex);
-    if (context->GetSwapChain()->OffscreenSize() != mEGLSurfaceSize
-      && !context->GetSwapChain()->Resize(mEGLSurfaceSize)) {
+    if (context->OffscreenSize() != mEGLSurfaceSize && 
    !context->ResizeOffscreen(mEGLSurfaceSize)) {
       return;
     }
   }

As you can see CompositeToDefaultTarget() isn't new. It's the way its resizing the screen that's causing problems. Interestingly this change has actually reverted back to the code as it is in ESR 78. So that means I can step through the ESR 78 version to compare the two.

But, my time in this coffee shop is coming to an end, so that will be a task for my journey back to Cambridge this evening on the train.

[...]

I'm on the train again, heading back to Cambridge. My next task is to step through the CompositeToDefaultTarget() method on ESR 78. The question I want to know the answer to is whether it's making the call to context->OffscreenSize() or not.

Brace yourself for a lengthy debugging step-through. As I've typically come to say, feel free to skip this without losing the context.

Thread 40 "Compositor" hit Breakpoint 1, mozilla::embedlite::
    EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget (
    this=0x7f80a77420, aId=...)
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.cpp:137
137     {
(gdb) n
138       const CompositorBridgeParent::LayerTreeState* state = 
    CompositorBridgeParent::GetIndirectShadowTree(RootLayerTreeId());
(gdb) n
139       NS_ENSURE_TRUE(state && state->mLayerManager, );
(gdb) n
141       GLContext* context = static_cast<CompositorOGL*>(
    state->mLayerManager->GetCompositor())->gl();
(gdb) n
142       NS_ENSURE_TRUE(context, );
(gdb) n
143       if (!context->IsCurrent()) {
(gdb) n
146       NS_ENSURE_TRUE(context->IsCurrent(), );
(gdb) p context->mIsOffscreen
$1 = false
(gdb) n
3566    /home/flypig/Documents/Development/jolla/gecko-dev-project/gecko-dev/
    obj-build-mer-qt-xr/dist/include/GLContext.h: No such file or directory.
(gdb) n
156         ScopedScissorRect 	
Thread 37 &quot;Compositor&quot; hit Breakpoint 1, mozilla::embedlite::
    EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget (
    this=0x7fc8a01040, aId=...)
    at mobile/sailfishos/embedthread/EmbedLiteCompositorBridgeParent.cpp:143
143     {
(gdb) n
144       const CompositorBridgeParent::LayerTreeState* state = 
    CompositorBridgeParent::GetIndirectShadowTree(RootLayerTreeId());
(gdb) n
145       NS_ENSURE_TRUE(state && state->mLayerManager, );
(gdb) n
147       GLContext* context = static_cast<CompositorOGL*>(
    state->mLayerManager->GetCompositor())->gl();
(gdb) n
148       NS_ENSURE_TRUE(context, );
(gdb) n
149       if (!context->IsCurrent()) {
(gdb) n
152       NS_ENSURE_TRUE(context->IsCurrent(), );
(gdb) p context->mDesc.isOffscreen
$2 = true
(gdb) n
3598    ${PROJECT}/obj-build-mer-qt-xr/dist/include/GLContext.h: No such file 
    or directory.
(gdb) n
155         MutexAutoLock lock(mRenderMutex);
(gdb) n
156         if (context->OffscreenSize() != mEGLSurfaceSize && 
    !context->ResizeOffscreen(mEGLSurfaceSize)) {
(gdb) n

Thread 37 &quot;Compositor&quot; received signal SIGSEGV, Segmentation fault.
0x0000007ff28a8978 in mozilla::gl::GLScreenBuffer::Size (this=0x0)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h:290
290     ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/UniquePtr.h: No 
    such file or directory.
(gdb) 

So, for once, this is just as expected. On ESR 78 context->IsOffscreen() is returning false (as it should do), whereas on ESR 91 it's returning true.

Great! This is something with a very clear cause and which it should be possible to find a sensible fix for.

On ESR 91 the initial value of this flag is set to false:

struct GLContextDesc final : public GLContextCreateDesc {
  bool isOffscreen = false;
};

The only place it gets set to true is in the GLContextEGL::CreateEGLPBufferOffscreenContextImpl() method:

  auto fullDesc = GLContextDesc{desc};
  fullDesc.isOffscreen = true;

In contrast on ESR 78 it's set in the constructor:

GLContext::GLContext(CreateContextFlags flags, const SurfaceCaps& caps,
                     GLContext* sharedContext, bool isOffscreen,
                     bool useTLSIsCurrent)
    : mUseTLSIsCurrent(ShouldUseTLSIsCurrent(useTLSIsCurrent)),
      mIsOffscreen(isOffscreen),
[...]

This constructor is called in two places. In GLContextEGL():

already_AddRefed<GLContextEGL> GLContextEGL::CreateGLContext(
    GLLibraryEGL* const egl, CreateContextFlags flags, const SurfaceCaps& caps,
    bool isOffscreen, EGLConfig config, EGLSurface surface, const bool useGles,
    nsACString* const out_failureId) {
[...]
  RefPtr<GLContextEGL> glContext =
      new GLContextEGL(egl, flags, caps, isOffscreen, config, surface, context);
[...]

And in CreateWrappingExisting():

already_AddRefed<GLContext> GLContextProviderEGL::CreateWrappingExisting(
    void* aContext, void* aSurface, void* aDisplay) {
[...]
  RefPtr<GLContextEGL> gl =
      new GLContextEGL(egl, CreateContextFlags::NONE, caps, false, config,
                       (EGLSurface)aSurface, (EGLContext)aContext);
[...]

The latter always ends up setting it to false as you can see. The former can also come from that route, but could also be arrived at via different routes, all of them in GLContextProviderEGL.cpp. It could be called from GLContextEGLFactory::CreateImpl() where it's always set to false:

already_AddRefed<GLContext> GLContextEGLFactory::CreateImpl(
    EGLNativeWindowType aWindow, bool aWebRender, bool aUseGles) {
[...]
  RefPtr<GLContextEGL> gl = GLContextEGL::CreateGLContext(
      egl, flags, caps, false, config, surface, aUseGles, &discardFailureId);
[...]

Or it could be called from GLContextEGL::CreateEGLPBufferOffscreenContextImpl() where it's always set to true:

/*static*/
already_AddRefed<GLContextEGL>
GLContextEGL::CreateEGLPBufferOffscreenContextImpl(
    CreateContextFlags flags, const mozilla::gfx::IntSize& size,
    const SurfaceCaps& minCaps, bool aUseGles,
    nsACString* const out_failureId) {
[...]
  RefPtr<GLContextEGL> gl = GLContextEGL::CreateGLContext(
      egl, flags, configCaps, true, config, surface, aUseGles, out_failureId);
[...]

The last of these is the only place where it can be set to true. So that's the one we have to focus on. Either there needs to be some more refined logic in there, or it shouldn't be going down that route at all.

This gives me something to dig into further tomorrow; but my train having arrived I'm going to have to leave it there for today. More tomorrow!

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