flypig.co.uk

Gecko-dev Diary

Starting in August 2023 I'll be upgrading the Sailfish OS browser from Gecko version ESR 78 to ESR 91. This page catalogues my progress.

Latest code changes are in the gecko-dev sailfishos-esr91 branch.

There is an index of all posts in case you want to jump to a particular day.

Gecko RSS feed Click the icon for the Gecko-dev Diary RSS feed.

Gecko

5 most recent items

27 Apr 2024 : Day 229 #
As discussed yesterday, today I'm investigating GLScreenBuffer::Swap(). A bit of debugging has demonstrated that it's hit frequently during the WebView render loop and it seems pretty integral. I also notice that there's already some commented-out code in there for reading off and debug-printing some pixel colours.

My plan was to do the same: read off some pixel colours, print them to the console, see whether they're updating as they should be. So the fact there's already some code in there to do this is an encouraging sign.

Unfortunately the code that's there doesn't work. Here's a snippet of the code in question:
    // uint32_t srcPixel = ReadPixel(src);
    // uint32_t destPixel = ReadPixel(dest);
    // printf_stderr("Before: src: 0x%08x, dest: 0x%08x\n", srcPixel,
    // destPixel);
It looks like a pretty slick implementation — there's not much to it after all — so using this would be ideal. Unfortunately it won't compile. As you can see it references a method called ReadPixel(), but this method doesn't actually exist. It's not even clear to me what the method is supposed to do, so replicating it isn't an option either.

Consequently I've grabbed the code that I used earlier in the CompositorOGL::EndFrame() method, transferred it over to this Swap() method and tweaked it for the new context. The new code looks like this:
  int xpos = size.width / 2;
  int ypos = size.height / 2;
  size_t bufferSize = sizeof(char) * 4;
  uint8_t* buf = static_cast<uint8_t*>(calloc(sizeof(uint8_t), bufferSize));
  bool result = ReadPixels(xpos, ypos, 1, 1, LOCAL_GL_RGBA, 
    LOCAL_GL_UNSIGNED_BYTE, buf);
  int pos = 0;

  volatile char red = buf[pos];
  volatile char green = buf[pos + 1];
  volatile char blue = buf[pos + 2];
  volatile char alpha = buf[pos + 3];

  printf_stderr(&quot;Colour before: (%d, %d, %d, %d), %d\n&quot;, red, green, 
    blue, alpha, result);
As you can see, it grabs a pixel from the surface and prints the colour value to the console. This is the "before" version and there's also an "after" version which does the same right after a call to SharedSurface::ProdCopy() has been made which appears to make a copy of the front buffer on to the back buffer. That's to match the commented-out code that was there before.

Now, having built and run the application, I don't get any output. Debugging shows that the Swap() method is definitely being hit. But the code I added to sample the surface is never called. Stepping through the code the reason immediately becomes obvious: the section of code I added them to is wrapped in a condition:
  if (mCaps.preserve && mFront && mBack) {
    auto src = mFront->Surf();
    auto dest = mBack->Surf();
[...]
  }
I just assumed that the code that was there before was a good place to add the sampling code, but mCaps.preserve is set to false so this condition is never being met.

So I've now moved the new sampling code outside of this condition, rebuilt and installed.

When I run the new library, the code is now definitely being executed, but the sampling is showing a very uniform output:
[...]
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
Colour before: (0, 0, 0, 0), 0
Colour after: (0, 0, 0, 0), 0
[...]
This can only be for one of two reasons: either the sampling code is broken, or the transfer of the image to the surface is broken. To understand which is happening we can consider the extra hint that's there in the output: the last value on the line is giving the result value of the call to ReadPixels(). In other words this indicates whether or not the ReadPixels() call was successful. The value being returned is always zero, equivalent of false. So no, the answer is that the call is not being successful.

Stepping through the code shows why. Inside the GLScreenBuffer::ReadPixels() method there's a call to actual do the work that looks like this:
    return surf->ReadPixels(x, y, width, height, format, type, pixels);
There are many different types of surface with potentially different overrides for ReadPixels() that this could be calling, but in this specific case, stepping through takes us to this method:
  virtual bool ReadPixels(GLint x, GLint y, GLsizei width, GLsizei height,
                          GLenum format, GLenum type, GLvoid* pixels) {
    return false;
  }
As we can see, this implementation of the call makes no attempt to read the pixels and is guaranteed to return false. Now even though this is a SharedSurface method it doesn't follow that the surface is actually of that type. For example SharedSurface_EGLImage is a subclass of SharedSurface, but since it doesn't override this particular method, it'll use the underlying SharedSurface method instead.

The debugger is also showing it as a SharedSurface type, but I'm suspicious. But I'm also tired, so I'm going to return to this tomorrow morning. With any luck, it'll be possible to unravel what's happening here and fix it.

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

Comments

Uncover Disqus comments