flypig.co.uk

List items

Items from the current list are shown below.

Blog

4 Sep 2023 : Day 19 #
If you've been following along you'll know that yesterday — indeed for the last couple of days — I've been grappling with a rendering issue: the GLScreenBuffer class which until now has been a key class in the EmbedLite rendering pipeline, has been removed.

Today I patched up all those GLScreenBuffer errors, replacing them with instances of SwapChain and rerouting all of the indirections appropriately. I just followed the steps that I described yesterday. Most of the errors related to this.

It all took quite a long time though, so there's not so much to write about today. It's counter-intuitive: the more awkward the changes the less there is to write about.

At this point it all looks sensible at any rate, but I've not yet built anything to check. Even when I have I'm not expecting these changes to actually work, which may sound odd, but given there's no way to test them until a build goes through, it's the only realistic assumption to make.

The last two errors left are unrelated to these changes though. These errors state that SchedulePauseOnCompositorThread() and ScheduleResumeOnCompositorThread() don't exist.

Looking at the relevant files they've certainly been removed. I want to know the diff that removed them.

Unfortunately git blame — my usual goto-tool for discovering how something changed — won't help here: git blame is great for finding things that have been added; not so great for finding things that have been removed. Instead I'll use git log to search through the diffs for the removal. The magical -S parameter, which searches for a phrase across the entire diff, is what we need. 
$ git log -1 -S SchedulePauseOnCompositorThread gfx/layers/ipc/CompositorBridgeParent.h
commit 7120835c49546a56f2781378d6a5e497cb860953
Author: sotaro 
Date:   Thu Nov 12 08:44:25 2020 +0000

    Bug 1676576 - Remove unused functions of CompositorBridgeParent r=nical
    
    Differential Revision: https://phabricator.services.mozilla.com/D96674
Following the details in this log message, the upstream Bugzilla bug report for this doesn't contain any helpful summary or explanation. There is a bug marked as a duplicate which adds this marginally-more-helpful context:
It doesn't seem to be called and can be removed.
I'm not seeing anything that replicates the functionality to help us out here. But there is a narrative that makes sense. It's common to remove unused functions, and if it's only EmbedLite that happens to need them, there's always the danger they'll get removed with minimal discussion upstream. Essentially the only check the upstream devs are likely to make is that everything still compiles.

So it seems that the best course of action may be to restore them. The changes are pretty self-contained, so the revert goes off smoothly: 
$ git revert 7120835c4
Auto-merging gfx/layers/ipc/CompositorBridgeParent.cpp
Auto-merging gfx/layers/ipc/CompositorBridgeParent.h
With those changes all made, it's finally time to kick off a build again, after three days of coding "blind". Let's see what happens. I'm not actually expecting all the changes I made to be syntactically correct, but there's always hope!

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

Comments

Uncover Disqus comments

Actions

Login Email Share

Archives

View