flypig.co.uk

I'm picking up where I left off, reintroducing code around the SharedSurface_Basic class. I've added back in the missing methods and code that ESR 78 includes as part of SharedSurface_Basic:

1. Create(). There's already an existing Create() method, but this new implementation provides a new signature.
2. Wrap(). This was missing from the ESR 91 version. It feels like we'll need it
3. Cast(). Similarly this might turn out to be needed.
4. SharedSurface_Basic(). Again, there was already a constructor but this one provides some new features.
5. ~SharedSurface_Basic(). In ESR 91 the destructor had been removed.
6. LockProdImpl(). This is an override; it's not clear that it's needed.
7. UnlockProdImpl(). Similarly here.
8. ProducerAcquireImpl(). And here.
9. ProducerReleaseImpl(). And also here.
10. ProdTexture(). However this override we need; the fact this was missing from ESR 91 is the reason we saw a segfault.

As well as adding all of these I also had to tweak some other code, adding an alternative SharedSurface constructor, along with the following new member variables:

1. const GLuint mTex.
2. const bool mOwnsTex.
3. GLuint mFB.

Having done all this I must now make sure that the new functionality is being used. That means finding out where the SharedSurface_Basic::Create() method is called and replacing it with our new version. Plus I'll need to check whether SharedSurface_Basic::Wrap() is used anywhere in ESR 78 and if so, see whether it should also be applied in ESR 91.

The good news is that with SharedSurface_Basic::Create() being a static function, it'll need to be fully qualified (in other words, prefixed with the class name) which makes it a lot easier to search the code for. On ESR 78 we have these cases:

$ grep -rIn "SharedSurface_Basic::Create(" *
gecko-dev/gfx/gl/SharedSurfaceGL.h:82:
    return SharedSurface_Basic::Create(mGL, mFormats, size, hasAlpha);
gecko-dev/gfx/gl/SharedSurfaceGL.cpp:22:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Create(
gecko-dev/gfx/gl/SharedSurface.cpp:41:
    tempSurf = SharedSurface_Basic::Create(gl, factory->mFormats, src->mSize,

Comparing that to the instances in ESR 91, we can see they're surprisingly similar:

$ grep -rIn "SharedSurface_Basic::Create(" *
gecko-dev/gfx/gl/SharedSurfaceGL.h:72:
    return SharedSurface_Basic::Create(desc);
gecko-dev/gfx/gl/SharedSurfaceGL.cpp:21:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Create(
gecko-dev/gfx/gl/SharedSurfaceGL.cpp:57:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Create(
gecko-dev/gfx/gl/SharedSurface.cpp:66:
    tempSurf = SharedSurface_Basic::Create({{gl, SharedSurfaceType::Basic,

On ESR 91 there are two different versions of SharedSurface_Basic::Create() because we just added a new one. But there are also differences in the way the method is called and I'd like to fix that so that the ESR 91 code better aligns with the ESR 78 code.

I've now changed the ESR 91 code so that it uses the same Create() override as the single version available in ESR 78. I'm really hoping to match up this particular set of functionality as closely as possible.

Finally I need to do a similar check for the SharedSurface_Basic::Wrap() method. This is also static, which again helps with discovery.

$ grep -rIn "SharedSurface_Basic::Wrap(" *
gecko-dev/gfx/gl/SharedSurfaceGL.cpp:44:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Wrap(GLContext* gl,
$ grep -rIn "Wrap(" gecko-dev/gfx/gl/SharedSurfaceGL.h
38:
    static UniquePtr<SharedSurface_Basic> Wrap(GLContext* gl,
$ grep -rIn "Wrap(" gecko-dev/gfx/gl/SharedSurfaceGL.cpp 
44:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Wrap(GLContext* gl,

The Wrap() method isn't an override, so this should be enough to demonstrate that the method isn't actually being used at all in ESR 78. So there should be no need to worry too much about it in ESR 91. And in fact once things are working it should almost certainly be removed. But I want to get a working executable before worrying about that.

Since I've only just now added the Wrap() method back in to ESR 91 there's no point doing a search there. All we'll find is the new code I just added. But let's do it anyway for completeness.

$ grep -rIn "SharedSurface_Basic::Wrap(" *
gecko-dev/gfx/gl/SharedSurfaceGL.cpp:79:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Wrap(GLContext* gl,
$ grep -rIn "Wrap(" gecko-dev/gfx/gl/SharedSurfaceGL.h
26:
    static UniquePtr<SharedSurface_Basic> Wrap(GLContext* gl,
$ grep -rIn "Wrap(" gecko-dev/gfx/gl/SharedSurfaceGL.cpp
79:
    UniquePtr<SharedSurface_Basic> SharedSurface_Basic::Wrap(GLContext* gl,

Having made these changes the partial builds have now all gone through, so it's time to set off the full build so I can test the executable again tomorrow.

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