flypig.co.uk

During the week of November 13th I'll be taking part in a Book Dash for The Turing Way. A book dash is a bit like a design sprint, where a collection of users commit to making a concerted effort to work on contributions for a short period of time; in this case a week.

The Turing Way is a bit like the bible of reproducible science and data science in particular. It calls itself the "handbook to reproducible, ethical and collaborative data science" and I'm a bit of a fan.
 
Until now I've not made the effort to try to contribute to it, but there are a number of areas where I'd like to.
 
In particular I've always had a bit of a problem with code review. I've been involved with code reviews for most of my adult life (since 2002 at least) and it's always been a rocky relationship. Jolla had a particular approach to code review that combined elements from both open source and commercial approaches. That's also helped shaped my opinion on it.

Given how challenging I've always found code review (as both reviewer and recipient) I really wanted to add something to The Turing Way about the power structures of code review. I'd like to think it might help others negotiate this tricky activity.

So that I can give the Book Dash the focus it deserves I've decided I'm going to have to pause my Gecko work while the dash is ongoing. I also need some time for preparation, so that means there are unlikely to be any Gecko Dev Diary entries between 4th and 19th November. On the weekend of the 18th I'll write a post about my experience on the Book Dash, and then after that my focus will be back on Gecko again.

This might slow down ESR 91 development. But it might also speed it up: I think it will be good for me to take a step back and mull things over. If I do come up with anything during the gap I'll be certain to write about it here. But if there's some silence for a bit, please don't be surprised!

And if anyone is particularly interested in this, feel free to join the celebrations on the last day of the book dash.

[...]

But let's get back to the code. Yesterday we followed the ESR 78 build down into the rendering depths of the rectangle fill algorithm. We eventually reached the following method which essentially blits a set of values into memory to represent the rectangle.

    template 
    static void rect_memsetT(T buffer[], T value, int count, size_t rowBytes, int height) {
        while (height --> 0) {
            memsetT(buffer, value, count);
            buffer = (T*)((char*)buffer + rowBytes);
        }
    }

The question I'm asking today is: does the same thing happen on ESR 91? This is as far as we can dig in this direction, so if we find it's doing the same thing, it would suggest that the rendering problem is happening at the other end of the process: the part where the surface gets rendered to the screen rather than the part where items get rendered to the surface.

For ESR 78 I had to step through the code quite carefully. That's because we didn't know where we were heading. Armed with the knowledge of what happens with ESR 78 I'm hoping we can short-circuit the process on ESR 91 by simply sticking a breakpoint on the rect_memsetT method to see whether it gets hit.

Unfortunately when I try to create the breakpoint there's nothing for it to attach to, maybe because it's template code. I've added a breakpoint to SkOpts::rect_memset32() instead. This is the method that calls rect_memsetT. This sticks, but will it be triggered?

It does get triggered and gives us an incredibly lengthy but otherwise clean backtrace.

Thread 8 "GeckoWorkerThre" hit Breakpoint 2, neon::rect_memset32
    (buffer=0x7ee07e6010, value=4294967295, count=1080, rowBytes=4320, height=1080)
    at gfx/skia/skia/src/opts/
    SkUtils_opts.h:57
57              rect_memsetT(buffer, value, count, rowBytes, height);
(gdb) bt
#0  neon::rect_memset32 (buffer=0x7ee07e6010, value=4294967295, count=1080,
    rowBytes=4320, height=1080)
    at gfx/skia/skia/src/opts/SkUtils_opts.h:57
#1  0x0000007fbc7078b4 in antifilldot8 (L=0, T=, R=276480,
    B=276480, blitter=0x7f9f3ceda8, fillInner=fillInner@entry=true)
    at gfx/skia/skia/src/core/SkScan_Antihair.cpp:683
#2  0x0000007fbc7079c4 in antifillrect (xr=..., blitter=)
    at gfx/skia/skia/src/core/SkScan_Antihair.cpp:697
#3  0x0000007fbc707ad0 in antifillrect (r=..., blitter=blitter@entry=0x7f9f3ceda8)
    at gfx/skia/skia/src/core/SkScan_Antihair.cpp:768
#4  0x0000007fbc708a54 in SkScan::AntiFillRect (origR=..., clip=0x7f889ae190,
    blitter=0x7f9f3ceda8)
    at gfx/skia/skia/src/core/SkScan_Antihair.cpp:791
#5  0x0000007fbc709090 in SkScan::AntiFillRect (r=..., clip=...,
    blitter=)
    at gfx/skia/skia/src/core/SkRasterClip.h:76
#6  0x0000007fbc754b20 in SkDraw::drawRect (this=this@entry=0x7f9f3cfb78,
    prePaintRect=..., paint=..., paintMatrix=paintMatrix@entry=0x0, 
    postPaintRect=postPaintRect@entry=0x0) at gfx/skia/skia/src/core/SkDraw.cpp:677
#7  0x0000007fbc719800 in SkDraw::drawRect (paint=..., rect=..., this=0x7f9f3cfb78)
    at gfx/skia/skia/src/core/SkDraw.h:42
#8  SkBitmapDevice::drawRect (this=, r=..., paint=...)
    at gfx/skia/skia/src/core/SkBitmapDevice.cpp:365
#9  0x0000007fbc72903c in SkCanvas::onDrawRect (this=, r=...,
    paint=...)
    at gfx/skia/skia/src/core/SkCanvas.cpp:2202
#10 0x0000007fbc720dec in SkCanvas::drawRect (this=0x7f889f0c60, r=...,
    paint=...)
    at gfx/skia/skia/include/core/SkRect.h:632
#11 0x0000007fba508ce8 in mozilla::gfx::DrawTargetSkia::FillRect
    (this=0x7f88978a00, aRect=..., aPattern=..., aOptions=...)
    at gfx/2d/DrawTargetSkia.cpp:852
#12 0x0000007fba53dd9c in mozilla::gfx::DrawTargetTiled::FillRect
    (this=this@entry=0x7f88c22350, aRect=..., aPattern=..., aDrawOptions=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:313
#13 0x0000007fbc15245c in nsDisplayCanvasBackgroundColor::Paint (this=0x7f88d5d6c0,
    aBuilder=, aCtx=)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/gfx/2D.h:124
#14 0x0000007fbc3a5584 in mozilla::FrameLayerBuilder::PaintItems
    (this=this@entry=0x7f88c1d6e0, aItems=std::vector of length 60,
    capacity 64 = {...}, aRect=..., aContext=aContext@entry=0x7f8856bbe0,
    aBuilder=aBuilder@entry=0x7f9f3d1268, aPresContext=aPresContext@entry=0x7f88a3d310, aOffset=..., 
    aXScale=1, aYScale=) at layout/painting/FrameLayerBuilder.cpp:7112
#15 0x0000007fbc3a59c4 in mozilla::FrameLayerBuilder::DrawPaintedLayer
    (aLayer=0x7f889802b0, aContext=0x7f8856bbe0, aRegionToDraw=...,
    aDirtyRegion=..., aClip=mozilla::layers::DrawRegionClip::DRAW,
    aRegionToInvalidate=..., aCallbackData=0x7f9f3d1268)
    at layout/painting/FrameLayerBuilder.cpp:7270
[...]
#62 0x0000007fb78b389c in ?? () from /lib64/libc.so.6

The question now is how does this compare with the "manual" backtrace we collected yesterday while following the ESR 78 code. Here's what we're comparing it to. This is greatly simplified because I only noted down a selection of the methods that were called.

static void rect_memsetT(T buffer[], T value, int count, size_t rowBytes, int height)
SkOpts::rect_memset32(device, color, width, rowBytes, height)
SkARGB32_Blitter::blitRect (height=45, width=777, y=0, x=256, this=0x7fde8d2038)
SkAAClipBlitter::blitRect (this=0x7fde8d1a58, x=29, y=0, width=1008, height=1080)
static void antifillrect(const SkXRect& xr, SkBlitter* blitter)
SkDraw::drawRect()
nsDisplayList::PaintRoot()
nsLayoutUtils::PaintFrame()
PresShell::Paint()

We see all of these there, apart from the calls to blitRect()

Thread 8 "GeckoWorkerThre" hit Breakpoint 2, neon::rect_memset32
    (buffer=0x7ee07e6010, value=4294967295, count=1080, rowBytes=4320,
    height=1080)
    at gfx/skia/skia/src/opts/SkUtils_opts.h:57
57              rect_memsetT(buffer, value, count, rowBytes, height);
(gdb) bt
#0  neon::rect_memset32 (buffer=0x7ee07e6010, value=4294967295, count=1080,
    rowBytes=4320, height=1080)
#2  0x0000007fbc7079c4 in antifillrect (xr=..., blitter=)
#6  0x0000007fbc754b20 in SkDraw::drawRect (this=this@entry=0x7f9f3cfb78,
    prePaintRect=..., paint=..., paintMatrix=paintMatrix@entry=0x0, 
#26 0x0000007fbc3a2acc in nsDisplayList::PaintRoot (this=this@entry=0x7f9f3d3078,
    aBuilder=aBuilder@entry=0x7f9f3d1268, aCtx=aCtx@entry=0x0, 
#27 0x0000007fbc12ee28 in nsLayoutUtils::PaintFrame
    (aRenderingContext=aRenderingContext@entry=0x0,
    aFrame=aFrame@entry=0x7f88bd0cd0, aDirtyRegion=..., 
#28 0x0000007fbc0b99ac in mozilla::PresShell::Paint
    (this=this@entry=0x7f88b8c360, aViewToPaint=aViewToPaint@entry=0x7f887beb20,
    aDirtyRegion=..., 

This is just one case and it may be that one of the other traces has a different call stack. So I worked through them all. There were cases that went a different route and ended up calling a method from the SKBlitter class. No calls to the blitRect() method though.

Thread 8 "GeckoWorkerThre" hit Breakpoint 2, neon::rect_memset32
    (buffer=0x7ee08a3f90, value=4293190837, count=824, rowBytes=4320,
    height=57)
    at gfx/skia/skia/src/opts/SkUtils_opts.h:57
57              rect_memsetT(buffer, value, count, rowBytes, height);
(gdb) bt
#0  neon::rect_memset32 (buffer=0x7ee08a3f90, value=4293190837, count=824,
    rowBytes=4320, height=57)
    at gfx/skia/skia/src/opts/SkUtils_opts.h:57
#1  0x0000007fbc6eb43c in SkBlitter::blitAntiRect (this=0x7f9f3cec28,
    x=, y=180, width=824, height=57, leftAlpha=, 
    rightAlpha=0 '\000') at gfx/skia/skia/src/core/SkBlitter.cpp:137
#2  0x0000007fbc792c58 in aaa_walk_convex_edges
    (prevHead=prevHead@entry=0x7f9f3cddc0, blitter=blitter@entry=0x7f9f3cde50,
    start_y=start_y@entry=179, 
    stop_y=stop_y@entry=238, leftBound=leftBound@entry=0,
    riteBound=riteBound@entry=70778880, isUsingMask=isUsingMask@entry=false)
    at gfx/skia/skia/include/private/SkTo.h:19
#3  0x0000007fbc79b980 in aaa_fill_path (forceRLE=false, isUsingMask=false,
    pathContainedInClip=true, stop_y=238, start_y=179, blitter=0x7f9f3cde50, 
    clipRect=..., path=...) at gfx/skia/skia/src/core/SkScan_AAAPath.cpp:1926
#4  SkScan::AAAFillPath (path=..., blitter=blitter@entry=0x7f9f3cec28, ir=...,
    clipBounds=..., forceRLE=forceRLE@entry=false)
    at gfx/skia/skia/src/core/SkScan_AAAPath.cpp:1988
#5  0x0000007fbc706b58 in SkScan::AntiFillPath (path=..., origClip=...,
    blitter=blitter@entry=0x7f9f3cec28, forceRLE=forceRLE@entry=false)
    at gfx/skia/skia/include/core/SkRegion.h:144
#6  0x0000007fbc707150 in SkScan::AntiFillPath (path=..., clip=...,
    blitter=0x7f9f3cec28)
    at gfx/skia/skia/src/core/SkRasterClip.h:76
#7  0x0000007fbc753dec in SkDraw::drawDevPath (this=this@entry=0x7f9f3cfb08,
    devPath=..., paint=..., drawCoverage=drawCoverage@entry=false, 
    customBlitter=customBlitter@entry=0x0, doFill=doFill@entry=true)
    at gfx/skia/skia/src/core/SkDraw.cpp:881
#8  0x0000007fbc754200 in SkDraw::drawPath (this=this@entry=0x7f9f3cfb08,
    origSrcPath=..., origPaint=..., prePathMatrix=prePathMatrix@entry=0x0, 
    pathIsMutable=pathIsMutable@entry=false,
    drawCoverage=drawCoverage@entry=false, customBlitter=customBlitter@entry=0x0)
    at gfx/skia/skia/src/core/SkTLazy.h:195
#9  0x0000007fbc719ec0 in SkDraw::drawPath (pathIsMutable=false,
    prePathMatrix=0x0, paint=..., path=..., this=0x7f9f3cfb08)
    at gfx/skia/skia/src/core/SkDraw.h:56
#10 SkBitmapDevice::drawPath (this=, path=..., paint=...,
    pathIsMutable=false)
    at gfx/skia/skia/src/core/SkBitmapDevice.cpp:401
#11 0x0000007fbc72a398 in SkCanvas::onDrawPath (this=0x7f889f0c60, path=...,
    paint=...)
    at gfx/skia/skia/src/core/SkCanvas.cpp:2365
#12 0x0000007fbc7213c4 in SkCanvas::drawPath (this=0x7f889f0c60, path=...,
    paint=...)
    at gfx/skia/skia/src/core/SkCanvas.cpp:1878
#13 0x0000007fba5094d8 in mozilla::gfx::DrawTargetSkia::Fill
    (this=0x7f88978a00, aPath=0x7f889aa920, aPattern=..., aOptions=...)
    at gfx/2d/PathSkia.h:84
#14 0x0000007fba538e5c in mozilla::gfx::DrawTargetTiled::Fill
    (this=0x7f88c22350, aPath=0x7f889aa920, aPattern=..., aDrawOptions=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/RefPtr.h:313
#15 0x0000007fba53a6d0 in mozilla::gfx::DrawTarget::FillRoundedRect
    (this=this@entry=0x7f88c22350, aRect=..., aPattern=..., aOptions=...)
    at gfx/2d/DrawTarget.cpp:190
[...]
#65 0x0000007fb78b389c in ?? () from /lib64/libc.so.6

In practice at this depth the code is identical between the two, so I'm going to put the differences down to compiler optimisation. At any rate I can't see any obvious way the rect_memset32() calls could be made without going through them.

This is enough for me. I'm going to call this a wrap and say that the sharp end of the rendering process — the point at which the individual items are rendered onto the backbuffer surface — appears to be pretty much identical between the two executables.

So it's time to switch focus and look at the other end of the process.

For the next steps I plan to check two things. First I want to step through GLContextProviderEGL::CreateWrappingExisting(), added in the following commit:

commit 51e18eb9b16ee94a775d3d4f1f333c11e306368e
Author: David Llewellyn-Jones 
Date:   Sat Sep 16 15:30:19 2023 +0100

    Restore GLContextProviderEGL::CreateWrappingExisting()
    
    Restores a method for creating a GLContexst for an existing EglDisplay.
    This is useful because on Sailfish OS the EglDisplay is created in
    QtMozEmbed and passed to xulrunner, rather than being created by
    xulrunner directly.

This was a change I made to allow the display surface to be passed from qtmozembed to the gecko code, since it had been removed from upstream. This is an obvious place where things might be going wrong.

So I'll step through both versions and see whether they differ.

Second, I need to work through all of the patches that have yet to be applied. It's quite possible there's something important in there that's missing and may be causing the rendering problems.

All of this is for the future though.

For all the other entries in my developer diary, check out the Gecko-dev Diary page.