1812202 - Screenshot can sometimes have transparent pixels

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[Niklas Baumgardner [:niklas]]

this happens on the screenshots component. we're not sure what causes this at the moment

Comment 1

[Sam Foster [:sfoster] (he/him)]

STR:

• In about:config, set screenshots.browser.component.enabled to true.
• Load a large/long page such as https://www.w3.org/TR/2011/WD-html5-20110405/Overview.html
• Use ctrl+shift+s or the context-menu to open the Screenshots UI
• Click the "Save full page" button
• Select the "Download" option and examine the resulting image

Expected:

• (after Bug 1743887 lands, you can expect an alert notification informing you the screenshot was capped to maximum dimensions)
• The image captures the full page in its entirety; there are no transparent pixels at the bottom of the image

Actual:

• The image is the correct dimensions, but only the first x pixels are filled with the captured screenshot, the rest is transparent pixels.

Comment 2

[Sam Foster [:sfoster] (he/him)]

In Bug 1743887 we clamp the capture area to the maximum dimensions, matching the maximum defined in https://searchfox.org/mozilla-central/rev/f40d29a11f2eb4685256b59934e637012ea6fb78/gfx/cairo/cairo/src/cairo-image-surface.c#62.

But it seem like this is either incorrect, or maybe there is an effective smaller cap on the total area/number of pixels we're not accounting for, causing us to lose pixel data along the way. I don't see any hard cap in cairo itself other than the max int value. But I'm not familiar with how we wrap that and the pipeline that gets data there and back from it. Presumably there's a buffer and its possible to exceed its size...

Do you have any thoughts or suggestions :jrmuizel?

Comment 3

[Niklas Baumgardner [:niklas]]

I was able to reproduce this on my windows 11 machine.
But this only occurs on certain zoom levels (150%, 175%, 225%). When I tried on 100%, 200%, and 250% I wasn't able to reproduce. There are other zoom levels I can try but I'm not really sure what the pattern is. I think it is worth noting that 250% is the default and recommended zoom level.

Comment 4

[Sam Foster [:sfoster] (he/him)]

Can you redirect the questions in Comment 2 :jimm?

Comment 5

[Jim Mathies [:jimm]]

Asking the triage crew in graphics, maybe they have some feedback.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

How big are the screenshots you're trying to make? 32767 is a fine limit. We should be able to draw screenshots that big. At what y coord does the transparency start?

Comment 7

[Bob Hood [:bhood]]

Tim, could you do some investigating here?

Comment 8

[Timothy Nikkel (:tnikkel)]

Hmm, since it depends on the fullzoom it doesn't seem like it is some kind of pixel limit. The visible rect we use for painting looks fine. And we do include the display items for the text that would be at the bottom of the screenshot. Those text items do get included in the recording that we make. And we seem to play back all of the recorded events in the recording. I'll see if I can trace the playback of one of the events that draws the text that fails to show up in the screenshot.

Comment 9

[Timothy Nikkel (:tnikkel)]

Hmm, this might be a pixel limit after all. It seems to stop drawing right around 16384 pixels, and if I switch the content backend we use (ie what we are playing back the recording into) from skia to cairo that fixes the bug.

I found this thread about what seems like some 16k limit in some cases in skia
https://groups.google.com/g/skia-discuss/c/M5jTzXJBDyI

Lee, do you know of any issues with skia that could be affecting us here?

Comment 10

[Lee Salzman [:lsalzman]]

Not distinctly. The Skia upstream issue you point to could very well be the problem, in which case there doesn't seem to be any magical upstream solution.

Comment 11

[Bob Hood [:bhood]]

Tim, can you think of any interim solution that can address this for now? Tiling would be the correct solution, but that's a major effort that we cannot address this semester.

Comment 12

[Ashley Hale [:ahale]]

Does Skia have any way to configure fixed-point precision or upgrade it to i64? 16384 is a common limitation of 16.16bit fixed point precision graphics engines, so we either need to tile (which seems like it may be a heavy lift) or tune something in skia (i.e. use i64).

Comment 13

[Lee Salzman [:lsalzman]]

Looks like upstream may have landed a fix for this only a couple days ago: https://bugs.chromium.org/p/skia/issues/detail?id=13626

If that is the case, it would make sense to just wait for the Skia update to progress, and I can try to rebase off the Skia milestone that would include that fix.

Comment 14

[Timothy Nikkel (:tnikkel)]

Thanks Lee! That's great news.

Comment 15

[Lee Salzman [:lsalzman]]

The current Skia milestone is m112, so I would expect that fix to be incorporated in m113.

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:bhood, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 17

[Sam Foster [:sfoster] (he/him)]

This seems to have gotten worse not better. When I follow the STR from comment #0 (I'm on linux) I get a image of the expected dimensions but now all the pixels are transparent.

Comment 18

[Bob Hood [:bhood]]

Sam, if you would, please attach the output of your about:support here when you get a chance.

When I can boot into it, I'll test with my Ubuntu 22.04 and Fx118 and see if I can repro.

Comment 19

[Timothy Nikkel (:tnikkel)]

I retested the problem that I was seeing/investigating here. Which to be concrete is the steps from comment 1 plus "move window to external monitor at 1x dpi, hit cmd + to increase full zoom one step". I can still reproduce the problem with a build before bug 1821512 (2023-04-01), but the problem does not reproduce with the latest nightly. So bug 1821512 did most likely fix the bug I was seeing, but the issue on linux is something else.

I was able to reproduce the linux issue (had to find a monitor for my old linux machine), it has been fully transparent on that machine since the new screenshot component has been working (around 2023-02-01).

Comment 20

[Sam Foster [:sfoster] (he/him)]

Attachment: about-support.txt

Attaching the about:support. This is a moz-central build from today.

I did some testing of the MAX_CAPTURE_DIMENSION we clamp to in ScreenshotsUtils and found:

• MAX_CAPTURE_DIMENSION = 32768 - I get the expected exceptions and failure to produce an image at all.
• MAX_CAPTURE_DIMENSION = 32767 - I get the fully-transparent image as documented in comment #0
• MAX_CAPTURE_DIMENSION = 32766 - I get the correct imge with no unexpected transparent pixels.

I am seeing quite a few [GFX3-]: Creating null Skia image from null SourceSurface logged in the terminal. I'm not sure if that is expected?

I'm happy to just land a patch which sets MAX_CAPTURE_DIMENSION = 32766 to fix this issue in Screenshots. But this does point to some kind of bug in the graphics layer I think. Let me know if you want to track that separately or use this bug.

Comment 21

[Sam Foster [:sfoster] (he/him)]

(In reply to Sam Foster [:sfoster] (he/him) from comment #20)

I did some testing of the MAX_CAPTURE_DIMENSION we clamp to in ScreenshotsUtils and found:

My STR for this was the same as comment #0. I'm waiting for the html5 document to fully load before capturing the screenshot. We pop up a notification if the max dimension was exceeded to inform the user we clamped it and they'll get a cropped screenshot. As you can see from the about:support attachment, I'm at 1:1 devicePixelRatio. I don't need to zoom or scale or anything to reproduce.

Comment 22

[Timothy Nikkel (:tnikkel)]

(In reply to Sam Foster [:sfoster] (he/him) from comment #20)

I am seeing quite a few [GFX3-]: Creating null Skia image from null SourceSurface logged in the terminal. I'm not sure if that is expected?

I haven't looked at this specific case, but that warning seems like something that is known and expected to happen so should probably just be removed: bug 1843748, comment 1.

Comment 23

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1812202 - Reduce the max capture dimension by 1px. r?niklas!

• Captures at exactly the max dimension (32767) result in a fully transparent image.
• Import these dimensions to use in tests so we have a single source of truth
• Update the same constant in the extension

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84af84f7d953
Reduce the max capture dimension by 1px. r=niklas

Comment 25

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/84af84f7d953

Comment 26

[Ina Popescu, Desktop QA]

I'm not sure if I've managed to reproduce the initial issue described in Comment 0. Could you please have a look over this video showing the screenshots taken in Nightly 113.0a1 and Nightly 121.0a1 on Ubuntu 22.04 and let me know if further testing is needed?
Comparing these screenshots, I've noticed that the one taken in Nightly 113.0a1 is not rendered properly in the Image Viewer. Thanks

Comment 27

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot 2023-11-16 at 12-04-33 HTML5.png

This is the screenshot image I get using today's nightly, following the STR from comment #0. It is exactly 32766 pixels tall, which is the revised max dimension that was changed in the patch we landed.

(In reply to Ina Popescu, Desktop QA from comment #26)

I'm not sure if I've managed to reproduce the initial issue described in Comment 0. Could you please have a look over this video showing the screenshots taken in Nightly 113.0a1 and Nightly 121.0a1 on Ubuntu 22.04 and let me know if further testing is needed?
Comparing these screenshots, I've noticed that the one taken in Nightly 113.0a1 is not rendered properly in the Image Viewer. Thanks

If I understand the video correctly, the results there seem directly opposite to what I would expect and have seen on my own system. Before the patch landed we would get transparent pixels in the full page screenshot, and with the patch I've not been able to reproduce the transparent pixels problem. I'm also on Ubuntu 22.04, though I guess there could be other factors at play. Would you mind re-testing :csasca?

Comment 28

[Ina Popescu, Desktop QA]

:sfoster , would it be possible to attach a screenshot with the defected behavior of screenshotting large page in older Nightly? I'll try to reproduce this issue again. Thanks.