flypig.co.uk

It's a bright new morning and I'm in a good mood! I've been working on the offscreen rendering pipeline since Day 159. That's a total of 87 days. It took me 28 days to get printing working; 33 days to get the browser rending working and 45 days to get the initial build to complete. So 87 days is the longest stretch of singly-directed work so far. It amounts to approximately three and a half weeks of full-time work.

Technically it's not all there yet and my task for the next few days is clear: I need to get my commits in order. I've made a large number of changes to get this working and my plan is to split them into two commits, which will eventually become two patches on top of the upstream Gecko code.

The first commit will be an equivalent to patch 0044, reverting the upstream D5005 changeset (Bug 1445570), but in a simplified form to make future maintenance easier.

The second wil lbe a patch to reintroduced GLScreenBuffer and essentially reverse the changes made in upstream changeset D75055 (Bug 1632249).

The second patch is going to be the hardest to get into shape because I had to make a lot of changes which I supsect can be simplified quite considerably, but I'm not certain. Calling the changes a reversel of D75055 is unfortunately oversimplifying things somehwat. It has the feel of a quite carefully balance stack of Jenga blocks, where removing a single piece might result in the entire stack coming tumblng down (which, for the avoidance of doubt, is a metaphor for the rendering not working again).

Unlike Jenga I have the benefit of git which will restack the blocks to a working position in an instant if things go awry. Nevertheless I want to get the lightest and neatest structure I can: the fewer changes in the patch now, the less maintenance will be needed in future.

That's all to come. Today I'm just going to worry about the first, much simpler, patch.

Having simplified it as much as possible, what are we left with? Well, here's the summary of the changes as provided by git:

 dom/base/nsDOMWindowUtils.cpp    |   1 -
 layout/base/nsDocumentViewer.cpp |   8 ++
 layout/base/nsPresContext.cpp    | 142 ++++++++++++++++++-------------
 layout/base/nsPresContext.h      |  33 +++++++
 layout/base/nsRefreshDriver.cpp  |   6 --
 5 files changed, 125 insertions(+), 65 deletions(-)

Let's compare that to the chnages from patch 0044:

 dom/base/nsDOMWindowUtils.cpp                 |   1 -
 layout/base/nsDocumentViewer.cpp              |   8 ++
 layout/base/nsPresContext.cpp                 | 132 ++++++++++--------
 layout/base/nsPresContext.h                   |  32 ++++-
 layout/base/nsRefreshDriver.cpp               |  43 ++----
 layout/base/nsRefreshDriver.h                 |   7 +-
 .../test/test_restyles_in_smil_animation.html |   4 +-
 7 files changed, 129 insertions(+), 98 deletions(-)

Well, it's a simplification. Not a huge simplification (190 changes vs. 227 changes), but a simplification nonetheless. More importantly, it's done.

With the easy patch dealt with it's now time to turn to the complex patch. Here's the current local commit history.

$ git log --oneline -5
555957f3a8ee (HEAD -> FIREFOX_ESR_91_9_X_RELBRANCH_patches) Restore 
    NotifyDidPaint event and timers
bc575f7ac075 Prevent errors from DownloadPrompter
06b15998d179 Enable dconf
95276b73d273 Restore GLScreenBuffer and TextureImageEGL
c6ea49286566 (origin/FIREFOX_ESR_91_9_X_RELBRANCH_patches) Disable SessionStore 
    functionality

I want to rearrange these so that commit 95276b73d273 is the most recent. That particular commit is the really big change made to get the renderer back working, as we can see from the change stats:

 dom/base/nsGlobalWindowOuter.cpp              |   6 +
 gfx/gl/AndroidSurfaceTexture.cpp              |   2 +-
 gfx/gl/GLContext.cpp                          | 217 +++++-
 gfx/gl/GLContext.h                            |  93 ++-
 gfx/gl/GLContextEGL.h                         |  10 +-
 gfx/gl/GLContextProviderEGL.cpp               | 284 +++++++-
 gfx/gl/GLContextProviderImpl.h                |  26 +
 gfx/gl/GLContextTypes.h                       |  11 +
 gfx/gl/GLLibraryEGL.cpp                       |  13 +-
 gfx/gl/GLLibraryEGL.h                         |   8 +-
 gfx/gl/GLScreenBuffer.cpp                     | 646 ++++++++++++++++++
 gfx/gl/GLScreenBuffer.h                       | 172 ++++-
 gfx/gl/GLTextureImage.cpp                     |  38 +-
 gfx/gl/SharedSurface.cpp                      | 321 ++++++++-
 gfx/gl/SharedSurface.h                        | 139 +++-
 gfx/gl/SharedSurfaceEGL.cpp                   | 113 ++-
 gfx/gl/SharedSurfaceEGL.h                     |  44 +-
 gfx/gl/SharedSurfaceGL.cpp                    | 152 ++++-
 gfx/gl/SharedSurfaceGL.h                      |  98 ++-
 gfx/gl/SurfaceTypes.h                         |  60 ++
 gfx/gl/TextureImageEGL.cpp                    | 221 ++++++
 gfx/gl/TextureImageEGL.h                      |  80 +++
 gfx/gl/moz.build                              |   1 +
 gfx/layers/CompositorTypes.h                  |   1 +
 gfx/layers/client/TextureClient.cpp           |  42 +-
 gfx/layers/client/TextureClient.h             |   3 +
 .../client/TextureClientSharedSurface.cpp     |  50 ++
 .../client/TextureClientSharedSurface.h       |  25 +
 gfx/layers/ipc/CompositorVsyncScheduler.cpp   |   2 -
 gfx/layers/opengl/CompositorOGL.cpp           |  35 +-
 gfx/thebes/gfxPlatform.cpp                    |   2 +-
 31 files changed, 2776 insertions(+), 139 deletions(-)

I'm anticipating that a lot of this will remain necessary, but if it's possible to cut it down, anything that can be removed will be a bonus. First though to rearrange the patches using an interactive rebase. Thankfully none of the commits overlap with these changes, so the rebase completes without a hitch. Here's how things look now.

$ git log --oneline -5
c3c263a98d03 (HEAD -> FIREFOX_ESR_91_9_X_RELBRANCH_patches) Restore 
    GLScreenBuffer and TextureImageEGL
d3ba4df29a32 Restore NotifyDidPaint event and timers
f55057391ac0 Prevent errors from DownloadPrompter
eab04b8c0d80 Enable dconf
c6ea49286566 (origin/FIREFOX_ESR_91_9_X_RELBRANCH_patches) Disable SessionStore 
    functionality

After carefully looking through the code I'm confident it could benefit from some changes. For example I'm fairly sure I added a bunch of code just for debugging purpose, such as when I was extracting images from the textures to help with debugging at various points. I should remove all that code.

Plus I suspect a bunch of the other methods were also added simply to align with the previous ESR 78 implementation. That doesn't mean they're actually needed and, in fact, some methods may not even get used at all. It's a hazard of the incremental approach I took.

The following lists the new methods that I added over the last few months. My plan is to add breakpoints to them, run the code and see which ones are hit. Any that aren't hit will then be candidates for removal.

GLContext::GuaranteeResolve()
GLContext::OffscreenSize()
GLContext::CreateScreenBuffer()
GLContext::CreateScreenBufferImpl()
GLContext::fBindFramebuffer()
GLContext::InitOffscreen()
CreateTexture()
CreateTextureForOffscreen()
GLContext::fBindFramebuffer()
GLContext::raw_fBindFramebuffer()
CreateTextureForOffscreen()
CreateTexture()
DefaultEglLibrary()
GLContextProviderEGL::CreateOffscreen()
GLScreenBuffer::Create()
GLScreenBuffer::CreateFactory()
GLScreenBuffer::CreateFactory()
GLScreenBuffer::GLScreenBuffer()
GLScreenBuffer::~GLScreenBuffer()
GLScreenBuffer::BindFB()
GLScreenBuffer::BindDrawFB()
GLScreenBuffer::BindReadFB()
GLScreenBuffer::BindReadFB_Internal()
GLScreenBuffer::GetDrawFB()
GLScreenBuffer::GetReadFB()
GLScreenBuffer::GetFB()
GLScreenBuffer::CopyTexImage2D()
GLScreenBuffer::ReadPixels()
GLScreenBuffer::Morph()
GLScreenBuffer::Attach()
GLScreenBuffer::Swap()
GLScreenBuffer::Resize()
GLScreenBuffer::CreateRead()
CreateRenderbuffersForOffscreen()
ReadBuffer::Create()
ReadBuffer::~ReadBuffer()
ReadBuffer::Attach()
ReadBuffer::Size()
SharedSurface::ProdCopy()
SharedSurface::SharedSurface()
SharedSurface::GetTextureFlags()
ChooseBufferBits()
SurfaceFactory::SurfaceFactory()
SurfaceFactory::~SurfaceFactory()
SurfaceFactory::NewTexClient()
SurfaceFactory::StartRecycling()
SurfaceFactory::StopRecycling()
SurfaceFactory::RecycleCallback()
SurfaceFactory::Recycle()
SharedSurface_EGLImage::SharedSurface_EGLImage()
SharedSurface_EGLImage::ReadPixels()
SurfaceFactory_Basic::SurfaceFactory_Basic()
SharedSurface_Basic::Wrap()
SharedSurface_Basic::SharedSurface_Basic()
SharedSurface_Basic::~SharedSurface_Basic()
SharedSurface_GLTexture::Create()
SharedSurface_GLTexture::~SharedSurface_GLTexture()
SharedSurface_GLTexture::ProducerReleaseImpl()
SharedSurface_GLTexture::ToSurfaceDescriptor()
GLFormatForImage()
GLTypeForImage()
TextureImageEGL::TextureImageEGL()
TextureImageEGL::~TextureImageEGL()
TextureImageEGL::DirectUpdate()
TextureImageEGL::BindTexture()
TextureImageEGL::Resize()
TextureImageEGL::BindTexImage()
TextureImageEGL::ReleaseTexImage()
TextureImageEGL::DestroyEGLSurface()
CreateTextureImageEGL()
TileGenFuncEGL()
SharedSurfaceTextureClient::SharedSurfaceTextureClient()
SharedSurfaceTextureClient::Create()
SharedSurfaceTextureClient::~SharedSurfaceTextureClient()

There are also a few datastructures which I should consider removing as well:

struct GLFormats
struct SurfaceCaps
enum class AttachmentType

An important part of these changes involves adding back the GLScreenBuffer method in to the code. But when the upstream change was made to remove this it was replaced with a class called SwapChain. So as part of all these changes I also want to see whether I can roll GLScreenBuffer into SwapChain. My gut tells me this would be a neat simplification if it's possible.

There are some changes that go along with this. I'm keeping a note of it here so I don't lose track of the things to consider.
 

• Combine SwapChain with GLScreenBuffer.
• Rename all GLContext::mScreen usage to GLContext::mSwapChain.
• Switch calls to Screen() with calls to GetSwapChain().

In addition to all of the changes above there are also some changes I need to take more care over. The following methods were added in order to accommodate Wayland rendering and so might be important for getting the browser working on native ports. However, in truth while related to offscreen rendering, these changes aren't directly required for it and I'm not able to test these changes myself. I therefore need to take care about whether these are really needed or not.

GetAppDisplay
_wl_compositor_create_surface
_wl_surface_destroy
LoadWaylandFunctions
UnloadWaylandFunctions
WaylandGLSurface::WaylandGLSurface
WaylandGLSurface::~WaylandGLSurface
CreateEmulatorBufferSurface

Finally many of these changes may be conditional on whether they're related to the Sailfish OS build or not. In this case I should consider wrapping some of them in a MOZ_WIDGET_QT precompiler conditional.

Phew, that's a lot to get through. But there is some kind of plan here and in practice I'm hoping it won't take as long to get through all of this as the lists above might suggest.

But this is enough for today. I'll start to look in to all these changes tomorrow and hopefully will immediately be able to start slimming down the changes.

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