Screenshot can sometimes have transparent pixels
Categories
(Core :: Graphics, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox120 | --- | fixed |
People
(Reporter: niklas, Assigned: sfoster)
References
Details
Attachments
(3 files)
this happens on the screenshots component. we're not sure what causes this at the moment
Assignee | ||
Comment 1•2 years ago
|
||
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.
Assignee | ||
Comment 2•2 years ago
|
||
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?
Reporter | ||
Comment 3•2 years ago
|
||
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.
Assignee | ||
Comment 4•2 years ago
|
||
Can you redirect the questions in Comment 2 :jimm?
Comment 5•2 years ago
|
||
Asking the triage crew in graphics, maybe they have some feedback.
Comment 6•2 years ago
|
||
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 8•2 years ago
|
||
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.
Updated•2 years ago
|
Comment 9•2 years ago
|
||
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•2 years ago
•
|
||
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•2 years ago
|
||
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•2 years ago
|
||
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•2 years ago
•
|
||
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 15•2 years ago
|
||
The current Skia milestone is m112, so I would expect that fix to be incorporated in m113.
Comment 16•2 years ago
|
||
The severity field is not set for this bug.
:bhood, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 17•1 year ago
|
||
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•1 year ago
|
||
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•1 year ago
|
||
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).
Assignee | ||
Comment 20•1 year ago
|
||
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 #0MAX_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.
Assignee | ||
Comment 21•1 year ago
|
||
(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•1 year ago
|
||
(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.
Assignee | ||
Comment 23•1 year ago
|
||
- 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
Updated•1 year ago
|
Comment 24•1 year ago
|
||
Comment 25•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Comment 26•1 year ago
|
||
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
Assignee | ||
Comment 27•1 year ago
|
||
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•1 year ago
|
||
: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.
Description
•