flypig.co.uk

Unsurprisingly the build I started lat night had completed by this morning. Unsurprising because it already passed through multiple partial builds yesterday. I've uploaded and installed the packages, now it's time to find out which methods are being called and which are not.

The approach I've devised for this is to start my test harbour-webview app running using the debugger. I'll then attach breakpoints to all of the methods I listed back Day 247, then execute the application and record which breakpoints get hit.

As each is hit I'll disable the breakpoint so it doesn't trigger multiple times. Eventually enough breakpoints will be disabled that the app will be running without interference from the debugger.

At that point I should have captured a pretty good list of which methods are necessary and which are unused.

I've ended up running this in three batches to keep things manageable. The result is that the breakpoints for all of the 39 methods listed below were triggered, meaning that these methods are definitely being used by the WebView renderer.

Batch 1:

1. GLContextProviderEGL::CreateOffscreen()
2. DefaultEglLibrary()
3. GLContext::fBindFramebuffer()
4. GLContext::raw_fBindFramebuffer()
5. GLContext::InitOffscreen()
6. GLContext::CreateScreenBuffer()
7. GLScreenBuffer::Create()
8. GLScreenBuffer::GLScreenBuffer()
9. CreateTextureForOffscreen()
10. CreateTexture()
11. GLContext::OffscreenSize()

Batch 2:

1. SurfaceFactory::SurfaceFactory()
2. ChooseBufferBits()
3. GLScreenBuffer::Resize()
4. SurfaceFactory::NewTexClient()
5. SharedSurface::SharedSurface()
6. SharedSurface::GetTextureFlags()
7. SurfaceFactory::StartRecycling()
8. GLScreenBuffer::Attach()
9. GLScreenBuffer::CreateRead()
10. ReadBuffer::Create()
11. CreateRenderbuffersForOffscreen()
12. GLScreenBuffer::BindFB()
13. GLScreenBuffer::Morph()
14. SurfaceFactory::~SurfaceFactory()
15. SurfaceFactory::StopRecycling()
16. ReadBuffer::Size()
17. GLScreenBuffer::Swap()
18. ReadBuffer::Attach()
19. SurfaceFactory::RecycleCallback()

Batch 3:

1. SurfaceFactory_Basic::SurfaceFactory_Basic()
2. SharedSurface_Basic::SharedSurface_Basic()
3. SharedSurfaceTextureClient::Create()
4. SharedSurfaceTextureClient::SharedSurfaceTextureClient()
5. SharedSurface_EGLImage::SharedSurface_EGLImage()
6. SharedSurfaceTextureClient::~SharedSurfaceTextureClient()
7. SharedSurface_Basic::~SharedSurface_Basic()
8. CreateTextureImageEGL()
9. TileGenFuncEGL()

The above methods all happened before the Web site had been fully rendered. The following were also hit, but a little later in the process; after rendering had apparently completed. I'm not sure whether this is really relevant, but I found it interesting:

1. TextureImageEGL::TextureImageEGL()
2. TextureImageEGL::BindTexture()
3. TextureImageEGL::Resize()
4. GLFormatForImage()
5. GLTypeForImage()
6. TextureImageEGL::DirectUpdate()
7. TextureImageEGL::~TextureImageEGL()
8. TextureImageEGL::ReleaseTexImage()
9. TextureImageEGL::DestroyEGLSurface()

That leaves the following methods that were added as a result of the changes I've made in order to get the WebView renderer working, but don't actually appear to be used by it. These are all candidates to be removed. I've actually marked the ones which I eventually did remove, but will come to those as we progress.

1. GLContext::GuaranteeResolve() - Removed
2. GLContext::CreateScreenBufferImpl() - Removed
3. GLScreenBuffer::CreateFactory() - Removed
4. GLScreenBuffer::~GLScreenBuffer()
5. GLScreenBuffer::BindDrawFB()
6. GLScreenBuffer::BindReadFB()
7. GLScreenBuffer::BindReadFB_Internal() - Removed
8. GLScreenBuffer::GetDrawFB() - Removed
9. GLScreenBuffer::GetReadFB() - Removed
10. GLScreenBuffer::GetFB() - Removed
11. GLScreenBuffer::CopyTexImage2D() - Removed
12. GLScreenBuffer::ReadPixels() - Removed
13. ReadBuffer::~ReadBuffer()
14. SharedSurface::ProdCopy() - Removed
15. SurfaceFactory::Recycle()
16. SharedSurface_EGLImage::ReadPixels() - Removed
17. SharedSurface_Basic::Wrap() - Removed
18. SharedSurface_GLTexture::Create() - Removed
19. SharedSurface_GLTexture::~SharedSurface_GLTexture() - Removed
20. SharedSurface_GLTexture::ProducerReleaseImpl() - Removed
21. SharedSurface_GLTexture::ToSurfaceDescriptor() - Removed
22. TextureImageEGL::BindTexImage()

I'm now working through the code, checking where each of the above methods is actually called, if it is at all. This is quite an intricate process because even if a method isn't called within gecko, that doesn't mean it's not exported and called in code that links to libxul.so, such as qtmozembed or the sailfish-components-webview code.

After carefully working through the list above, it looks like I should be able to safely remove the following nine methods:

1. GLContext::GuaranteeResolve() - Removed
2. GLContext::CreateScreenBufferImpl() - Removed
3. GLScreenBuffer::CreateFactory() - Removed
4. GLScreenBuffer::BindReadFB_Internal() - Removed
5. GLScreenBuffer::GetDrawFB() - Removed
6. GLScreenBuffer::GetReadFB() - Removed
7. GLScreenBuffer::GetFB() - Removed
8. GLScreenBuffer::CopyTexImage2D() - Removed
9. GLScreenBuffer::ReadPixels() - Removed

In addition to these, it looks like I'm also safe to remove the SurfaceCaps::preserve flag, since this is always set to false. Removing this flag also allows me to remove additional methods which are only ever called in the situations when this flag is set to true. So I've both removed the flag and simplified the code that's conditional on it.

Having made these changes I've built the library, installed it and run the test app again. The code built fine and everything appears to be working correctly, so these changes look to be safe. With these having gone through successfully, there are now some more methods which have become orphans as a result, so I can safely remove these as well:

1. SharedSurface::ProdCopy() - Removed
2. SharedSurface_EGLImage::ReadPixels() - Removed
3. SharedSurface_Basic::Wrap() - Removed

None of the methods from SharedSurface_GLTexture are being called either, so I'm wondering whether it would make sense to remove the entire class. But on doing a quick search I can see that there is a single place where a SharedSurface_GLTexture instance is created. This is in the EmbedLiteCompositorBridgeParent::PrepareOffscreen() method where the relevant code looks like this:

  if (context->GetContextType() == GLContextType::EGL) {
    // [Basic/OGL Layers, OMTC] WebGL layer init.
    factory = SurfaceFactory_EGLImage::Create(context, screen->mCaps, nullptr, 
    flags);
  } else {
    // [Basic Layers, OMTC] WebGL layer init.
    // Well, this *should* work...
    factory = MakeUnique<SurfaceFactory_GLTexture>(context, screen->mCaps, 
    nullptr, flags);
  }

Om the device I'm using for testing the context type is set to GLContextType::EGL and so it's always SurfaceFactory_EGLImage that's used to create the surface. But can I be sure there won't be situations in which the other branch will be executed? Perhaps on other devices with different hardware and drivers available?

The value returned by context->GetContextType() is determined based on the class context is an instance of. There are multiple classes that inherit from GLContext and so could potentially be in use here. They all derive from GLContext and override the GetContextType() method to generate different return values for this call. Here's an example from GLContextEGL:

  virtual GLContextType GetContextType() const override {
    return GLContextType::EGL;
  }

To find out whether anything other than GLContextType::EGL will ever be returned I need to find out how the context object is created. Unfortunately this is a bit of a maze. For example, this is how the context is pulled in for use inside the PrepareOffscreen() method:

  GLContext* context = static_cast<CompositorOGL*>(
    state->mLayerManager->GetCompositor())->gl();

Not pretty. The place where the context appears to be created is in CompositorOGL::CreateContext(). The logic there is also a bit serpentine, but at least for the WebView, it eventually leads to a call of the following:

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

That GLContextProvider::CreateOffscreen() actually goes through to GLContextProviderEGL::CreateOffscreen(). Why is that? That's because GLContextProvider provides only a macro which is actually implemented by GLContextProviderEGL. In fact, there are no other instances of CreateOffscreen() implemented by any other GLContextProvider types.

For completeness, CreateOffscreen() goes on to call the following, where as we progress through the list we go deeper down the call stack:

1. GLContextProviderEGL::CreateHeadless()
2. GLContextEGL::CreateEGLPBufferOffscreenContext()
3. GLContextEGL::CreateEGLPBufferOffscreenContextImpl()
4. GLContextEGL::CreateGLContext()
5. new GLContextEGL()

As you can see, the deepest of these calls will create a GLContextProvider with type GLContextType::EGL. So it really does look like the context will always be of type GLContextEGL when this runs on a Sailfish device. I've therefore decided to remove the SharedSurface_GLTexture class completely, along with all of its associated code (e.g. SurfaceFactory_GLTexture) since this will never get used. That means removing all of the following:

1. SharedSurface_GLTexture::Create() - Removed
2. SharedSurface_GLTexture::~SharedSurface_GLTexture() - Removed
3. SharedSurface_GLTexture::ProducerReleaseImpl() - Removed
4. SharedSurface_GLTexture::ToSurfaceDescriptor() - Removed

Now to build and test the result:

$ make -j1 -C obj-build-mer-qt-xr/gfx/
$ make -j16 -C `pwd`/obj-build-mer-qt-xr/toolkit

Having removed it the code compiles fine, but hits a problem during linkage:

aarch64-meego-linux-gnu-ld: libxul.so: hidden symbol 
    `_ZN7mozilla2gl23SharedSurface_GLTexture6CreateEPNS0_9GLContextERKNS0_9GL
    FormatsERKNS_3gfx12IntSizeTypedINS7_12UnknownUnitsEEEb' isn't defined
aarch64-meego-linux-gnu-ld: final link failed: bad value

The reason for the failure is that I've also made changes to EmbedLiteCompositorBridgeParent.cpp, but the commands I ran to recompile the code didn't incorporate this file. That's how these partial builds work: you have to be careful to include all relevant directories in the make commands. So I'll need to ask the compiler to do a bit more work.

$ make -j1 -C obj-build-mer-qt-xr/mobile/sailfishos/
$ make -j16 -C `pwd`/obj-build-mer-qt-xr/toolkit
$ strip obj-build-mer-qt-xr/toolkit/library/build/libxul.so

This time it builds successfully and I'm able to copy the resulting library over to my device. With this new version of the library installed both the browser and WebView app run successfully without any problems.

That's at least three edit-rebuild-test cycles we've been round today. More than enough for one day I'd say. Tomorrow I'll continue stripping out unused code from the offscreen rendering patch. It's still a very large patch, so if there's any more redundant code it'd be really good to get rid of it.

After having spent so long floundering around trying to fix offscreen rendering over the last few months, it's really nice to be making steady progress again. Solid; mundane; and steady; but progress nonetheless.

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