flypig.co.uk

List items

Items from the current list are shown below.

Gecko

2 Sep 2023 : Day 17 #
Before I get into the main post I wanted to make a note about burnout. Up until now I've been spending about two hours per day working on this. That's not a huge amount, but on top of my other responsibilities it's not insignificant. One of the nicest parts of this project has been waking up to the excitement of finding out whether the build has progressed or not.

This morning I just felt a bit exhausted. It's now been a week of me combining this Gecko development with my full-time job and I want to be careful not to end up suffering from burnout as a result. I don't think I'm overdoing it, but I do want to recognise the signs as they appear. This is my first indicator and I'll be taking care to notice any others.

But I'm also very conscious that if this is how it feels like for me, it must be exhausting for anyone who's read this far as well! If you have, then I take my hat off to you. But take care to look out for those warning signs too!

With that out of the way, let's get on to the development.

Yesterday we'd just removed the final annotation from the CompositorBridgeParent and we'd set the build off to see what effect this would have. Hopefully a positive one. But let's see...

It's another huge wave of errors, but most are the same as we got yesterday, apart — thankfully — from the error about CompositorBridgeParent begin final.

Unlike yesterday I'm going to include the list of errors in full today. I recommend you skip past it, maybe just take a look at the very first error, which is the one we'll want to look at first. But we'll want to refer back to the list later to tackle them all, so it's good to keep a record of them. 
188:01.52 mobile/sailfishos
188:10.53 In file included from ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:8:
188:10.53 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.h:58:16: error: ‘virtual void 
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget(
          mozilla::layers::PCompositorBridgeParent::VsyncId)’ marked ‘override’,
          but does not override
188:10.53    virtual void CompositeToDefaultTarget(VsyncId aId) override;
188:10.53                 ^~~~~~~~~~~~~~~~~~~~~~~~
188:11.03 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In constructor
          ‘mozilla::embedlite::EmbedLiteCompositorBridgeParent::
          EmbedLiteCompositorBridgeParent(uint32_t,
          mozilla::layers::CompositorManagerParent*,
          mozilla::CSSToLayoutDeviceScale, const TimeDuration&,
          const CompositorOptions&, bool, const IntSize&)’:
188:11.03 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:60:16: error: ‘AddBoolVarCache’
          is not a member of ‘mozilla::Preferences’
188:11.03    Preferences::AddBoolVarCache(&mUseExternalGLContext,
188:11.03                 ^~~~~~~~~~~~~~~
188:11.03 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::PrepareOffscreen()’:
188:11.03 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:111:5: error: ‘GLScreenBuffer’
          was not declared in this scope
188:11.03      GLScreenBuffer* screen = context->Screen();
188:11.04      ^~~~~~~~~~~~~~
188:11.06 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:111:5: note: suggested alternative:
          ‘SharedBuffer’
188:11.06      GLScreenBuffer* screen = context->Screen();
188:11.06      ^~~~~~~~~~~~~~
188:11.06      SharedBuffer
188:11.06 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:111:21: error: ‘screen’ was not
          declared in this scope
188:11.06      GLScreenBuffer* screen = context->Screen();
188:11.06                      ^~~~~~
188:11.06 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:111:21: note: suggested alternative:
          ‘nsScreen’
188:11.06      GLScreenBuffer* screen = context->Screen();
188:11.06                      ^~~~~~
188:11.06                      nsScreen
188:11.06 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:111:39: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘Screen’
188:11.06      GLScreenBuffer* screen = context->Screen();
188:11.06                                        ^~~~~~
188:11.06 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:126:30: error:
          ‘SurfaceFactory_GLTexture’ was not declared in this scope
188:11.07          factory = MakeUnique(context, screen->mCaps, nullptr, flags);
188:11.07                               ^~~~~~~~~~~~~~~~~~~~~~~~
188:11.14 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:126:30: note: suggested
          alternative: ‘SurfaceDescriptorSharedGLTexture’
188:11.14          factory = MakeUnique(context, screen->mCaps, nullptr, flags);
188:11.15                               ^~~~~~~~~~~~~~~~~~~~~~~~
188:11.15                               SurfaceDescriptorSharedGLTexture
188:11.15 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘virtual void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::CompositeToDefaultTarget(
          mozilla::layers::PCompositorBridgeParent::VsyncId)’:
188:11.15 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:150:18: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘OffscreenSize’; did you
          mean ‘IsOffscreen’?
188:11.15      if (context->OffscreenSize() != mEGLSurfaceSize && !context->ResizeOffscreen(mEGLSurfaceSize)) {
188:11.15                   ^~~~~~~~~~~~~
188:11.15                   IsOffscreen
188:11.15 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:150:66: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘ResizeOffscreen’; did
          you mean ‘IsOffscreen’?
188:11.15      if (context->OffscreenSize() != mEGLSurfaceSize && !context->ResizeOffscreen(mEGLSurfaceSize)) {
188:11.15                                                                   ^~~~~~~~~~~~~~~
188:11.15                                                                   IsOffscreen
188:11.15 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:156:5: error: ‘ScopedScissorRect’
          was not declared in this scope
188:11.15      ScopedScissorRect autoScissor(context);
188:11.15      ^~~~~~~~~~~~~~~~~
188:11.19 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::PresentOffscreenSurface()’:
188:11.19 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:180:3: error: ‘GLScreenBuffer’
          was not declared in this scope
188:11.19    GLScreenBuffer* screen = context->Screen();
188:11.19    ^~~~~~~~~~~~~~
188:11.21 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:180:3: note: suggested alternative:
          ‘SharedBuffer’
188:11.22    GLScreenBuffer* screen = context->Screen();
188:11.22    ^~~~~~~~~~~~~~
188:11.22    SharedBuffer
188:11.22 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:180:19: error: ‘screen’ was not
          declared in this scope
188:11.22    GLScreenBuffer* screen = context->Screen();
188:11.22                    ^~~~~~
188:11.22 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:180:19: note: suggested alternative:
          ‘nsScreen’
188:11.22    GLScreenBuffer* screen = context->Screen();
188:11.22                    ^~~~~~
188:11.22                    nsScreen
188:11.22 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:180:37: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘Screen’
188:11.22    GLScreenBuffer* screen = context->Screen();
188:11.22                                      ^~~~~~
188:11.22 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::GetPlatformImage(
          const std::function&)’:
188:11.22 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:222:3: error: ‘GLScreenBuffer’
          was not declared in this scope
188:11.22    GLScreenBuffer* screen = context->Screen();
188:11.22    ^~~~~~~~~~~~~~
188:11.24 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:222:3: note: suggested alternative:
          ‘SharedBuffer’
188:11.24    GLScreenBuffer* screen = context->Screen();
188:11.24    ^~~~~~~~~~~~~~
188:11.24    SharedBuffer
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:222:19: error: ‘screen’ was not
          declared in this scope
188:11.25    GLScreenBuffer* screen = context->Screen();
188:11.25                    ^~~~~~
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:222:19: note: suggested alternative:
          ‘nsScreen’
188:11.25    GLScreenBuffer* screen = context->Screen();
188:11.25                    ^~~~~~
188:11.25                    nsScreen
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:222:37: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘Screen’
188:11.25    GLScreenBuffer* screen = context->Screen();
188:11.25                                      ^~~~~~
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:232:19: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mType’
188:11.25    if (sharedSurf->mType == SharedSurfaceType::EGLImageShare) {
188:11.25                    ^~~~~
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:233:68: error: ‘Cast’ is not a
          member of ‘mozilla::gl::SharedSurface_EGLImage’
188:11.25      SharedSurface_EGLImage* eglImageSurf = SharedSurface_EGLImage::Cast(sharedSurf);
188:11.25                                                                     ^~~~
188:11.25 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:234:48: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mSize’
188:11.25      callback(eglImageSurf->mImage, sharedSurf->mSize.width, sharedSurf->mSize.height);
188:11.26                                                 ^~~~~
188:11.26 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:234:73: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mSize’
188:11.26      callback(eglImageSurf->mImage, sharedSurf->mSize.width, sharedSurf->mSize.height);
188:11.26                                                                          ^~~~~
188:11.26 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void*
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::GetPlatformImage(
          int*, int*)’:
188:11.26 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:250:3: error: ‘GLScreenBuffer’
          was not declared in this scope
188:11.26    GLScreenBuffer* screen = context->Screen();
188:11.26    ^~~~~~~~~~~~~~
188:11.27 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:250:3: note: suggested alternative:
          ‘SharedBuffer’
188:11.27    GLScreenBuffer* screen = context->Screen();
188:11.27    ^~~~~~~~~~~~~~
188:11.28    SharedBuffer
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:250:19: error: ‘screen’ was not
          declared in this scope
188:11.28    GLScreenBuffer* screen = context->Screen();
188:11.28                    ^~~~~~
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:250:19: note: suggested alternative:
          ‘nsScreen’
188:11.28    GLScreenBuffer* screen = context->Screen();
188:11.28                    ^~~~~~
188:11.28                    nsScreen
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:250:37: error: ‘class
          mozilla::gl::GLContext’ has no member named ‘Screen’
188:11.28    GLScreenBuffer* screen = context->Screen();
188:11.28                                      ^~~~~~
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:258:24: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mSize’
188:11.28    *width = sharedSurf->mSize.width;
188:11.28                         ^~~~~
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:259:25: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mSize’
188:11.28    *height = sharedSurf->mSize.height;
188:11.28                          ^~~~~
188:11.28 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:261:19: error: ‘class
          mozilla::gl::SharedSurface’ has no member named ‘mType’
188:11.28    if (sharedSurf->mType == SharedSurfaceType::EGLImageShare) {
188:11.29                    ^~~~~
188:11.29 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:262:68: error: ‘Cast’ is not a
          member of ‘mozilla::gl::SharedSurface_EGLImage’
188:11.29      SharedSurface_EGLImage* eglImageSurf = SharedSurface_EGLImage::Cast(sharedSurf);
188:11.29                                                                     ^~~~
188:11.29 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::SuspendRendering()’:
188:11.29 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:272:27: error:
          ‘SchedulePauseOnCompositorThread’ is not a member of
          ‘mozilla::layers::CompositorBridgeParent’
188:11.29    CompositorBridgeParent::SchedulePauseOnCompositorThread();
188:11.29                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
188:11.29 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp: In member function ‘void
          mozilla::embedlite::EmbedLiteCompositorBridgeParent::ResumeRendering()’:
188:11.29 ${PROJECT}/gecko-dev/mobile/sailfishos/embedthread/
          EmbedLiteCompositorBridgeParent.cpp:279:29: error:
          ‘ScheduleResumeOnCompositorThread’ is not a member of
          ‘mozilla::layers::CompositorBridgeParent’
188:11.29      CompositorBridgeParent::ScheduleResumeOnCompositorThread(mSurfaceOrigin.x,
188:11.29                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
188:12.91 make[4]: *** [${PROJECT}/gecko-dev/config/rules.mk:676: EmbedLiteCompositorBridgeParent.o] Error 1
At least some of these would seem to be covered by patch 0020 entitled "Allow compositor specializations to override the composite (part 2)". I do enjoy a good sequel. Applying the patch gives positive — but not complete — results: 
$ patch -d gecko-dev -p1 < rpm/0020-sailfishos-compositor-Allow-compositor-specializatio.patch 
patching file gfx/layers/ipc/CompositorBridgeParent.cpp
Hunk #1 succeeded at 874 (offset -50 lines).
patching file gfx/layers/ipc/CompositorBridgeParent.h
Hunk #1 succeeded at 740 with fuzz 2 (offset -27 lines).
patching file gfx/layers/ipc/CompositorVsyncScheduler.cpp
Hunk #1 FAILED at 246.
Hunk #2 succeeded at 289 with fuzz 2 (offset 7 lines).
1 out of 2 hunks FAILED -- saving rejects to file gfx/layers/ipc/CompositorVsyncScheduler.cpp.rej
patching file gfx/layers/ipc/CompositorVsyncSchedulerOwner.h
patching file gfx/layers/wr/WebRenderBridgeParent.cpp
Hunk #1 succeeded at 2680 with fuzz 1 (offset 193 lines).
patching file gfx/layers/wr/WebRenderBridgeParent.h
Hunk #1 succeeded at 183 (offset -22 lines).
The hunk that failed is actually just a single line failure, so easy to fix, but also worth delving into a little.

The relevant lines of the patch look like this: 
diff --git a/gfx/layers/ipc/CompositorVsyncScheduler.cpp b/gfx/layers/ipc/CompositorVsyncScheduler.cpp
index d00a7fae73ea..1fbd3c450ea3 100644
--- a/gfx/layers/ipc/CompositorVsyncScheduler.cpp
+++ b/gfx/layers/ipc/CompositorVsyncScheduler.cpp
@@ -246,8 +246,7 @@ void CompositorVsyncScheduler::Composite(VsyncId aId,
     mLastCompose = aVsyncTimestamp;
 
     // Tell the owner to do a composite
-    mVsyncSchedulerOwner->CompositeToTarget(aId, nullptr, nullptr);
-
+    mVsyncSchedulerOwner->CompositeToDefaultTarget(aId);
     mVsyncNotificationsSkipped = 0;
 
     TimeDuration compositeFrameTotal = TimeStamp::Now() - aVsyncTimestamp;
But the underlying variables have changed. In ESR 78 the aId parameter was passed into the method that this code belongs to. In ESR 91 this has been changed so that the entire VsyncEvent is passed in, rather than just its Id. The fix is therefore just to extract the Id from the event and pass that in instead. This is what we're left with: 
    // Tell the owner to do a composite
    mVsyncSchedulerOwner->CompositeToTarget(aVsyncEvent.mId, nullptr, nullptr);
With that done, the patch is now applied fully. Before kicking off the build I'm going to check ahead at some of these other errors, in case they're also easy to fix. They're all also coming from the EmbedLiteCompositorBridgeParent.cpp, but it doesn't look like they're related to patch 0020.

First we have an error about Preferences::AddBoolVarCache() not existing. In ESR 78 this method was defined in modules/libpref/Preferences.h. Checking the log for this file it's clear that this relates to a long running Mozilla quest to remove all VarCache machinery from Gecko. The relevant diff is D79538. The history seems to go back over six years with the desire to remove CacheData:
  1. Bug 1642727: Delete all VarCache code
  2. Bug 1570212: Convert various VarCache prefs to static prefs
  3. Bug 1569526: Remove CacheData
  4. Bug 1448219: [meta] Convert all VarCache prefs to use StaticPrefs
  5. Bug 1436655: Introduce a mechanism for prefs to be defined entirely in the binary
The last of the items in this list highlights some of the advantages of moving away from VarCache. I've picked out a few of them here:
 
  • It eliminates the duplication (in all.js and the Add*VarCache() call) of the pref name and default value, preventing potential mismatches. (This is a real problem in practice!)
  • There is now a single initialization point for these VarCache prefs.
    • This avoids need to find a place to insert the Add*VarCache() calls, which are currently spread all over the place.
    • It also eliminates the common pattern whereby these calls are wrapped in a execute-once block protected by a static boolean (see bug 1346224).
    • And it's no longer possible to have a VarCache pref for which only one of the pieces has been setup. [...]
    • (Future work) This will allow the pref names to be stored statically, saving memory in every process.

The EmbedLite code (the stuff that isn't upstream) uses VarCache quite a bit: 14 uses for 12 additional configuration options. So all of these will need changing. At this point I thought about changing all of the values to use the static pref approach, following the example of media.video_stats.enabled — as shown in diff D40340 — and trying to apply the same to the EmbedLite code.

But then I had a useful discussion with Raine over IRC. Not only did it give me a much-needed motivational boost, it also brought clarity too. Clarity on the most suitable approach for these preferences.

Most likely these will need to be switched to static prefs. But in the meantime, while we're still just getting a working build, I can set them to suitable default values. It's not essential that we can set these until we have something actually running.

It's important not to lose sight of the goal here: a working build as quickly as possible.

So, now I've switched all instances of AddBoolVarCache to just setting the values of the relevant variable. For example, from this: 
Preferences::AddBoolVarCache(&sUseExternalGLContext,
    "embedlite.compositor.external_gl_context", false);
To this: 
sUseExternalGLContext = false; // "embedlite.compositor.external_gl_context"
Next is a cascade of errors related to GLScreenBuffer. These seem to have been caused by the complete removal of GLScreenBuffer as detailed in Bugzilla bug 1632249 and diff D75055. This looks like it could be a real problem, at the very least requiring some work to resolve. It's a huge upstream change and we really need what's in GLScreenBuffer and now it's completely gone.

This is quite a traumatic realisation. As I write this, it's not at all clear to me how to fix this; I'll have to spend some hours poring over the existing ESR 78 code, the new ESR 91 code, and the diff between them.

Some of the things I notice are that GLSCreenBuffer, mScreen and SharedSurface::mSize all seem to be missing. There are also errors related to SurfaceFactory_GLTexture and various Offscreen methods and GLContext. That's plenty to be getting on with.

This takes me into the night, too late to make it worthwhile starting a meaningful build. I'll have to continue looking at these errors tomorrow.

For all the other posts, check out my full Gecko Dev Diary.

Comments

Uncover Disqus comments

Actions

Login Email Share

Archives

View