1544390 - GeckoView: Thumbnail screenshots contain scrollbar

Status: RESOLVED WONTFIX

(GeckoView :: General, defect, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

AC issue: https://github.com/mozilla-mobile/android-components/issues/2701

See screenshot in AC issue: The thumbnails returned by GeckoView can contain the scrollbar of the view.

Comment 1

[Chris Peterson [:cpeterson]]

Emily said she would take a look at this screenshot bug.

Comment 2

[Emily Toop (:fluffyemily)]

In investigating this issue it has become clear that the constraints of Webrender and the compositor means that it is not possible to remove the scrollbars from screenshots using the current screenshotting strategy. An investigation into using the existing Fennec APIs to do the screenshot highlighted that the APIs cannot take screenshots of video and webgl and result in black holes where the content should be.

In discussion with :snorp we decided that the presence of scrollbars is preferable to the presence of black holes in screenshots. It is recommended that users of the screenshot API either a) delay the taking of screenshots slightly until the scrollbars have disappeared from the screen, or crop the screenhots to remove the right and bottom after the image has been returned from the API.

Below I include the conversation between myself, :snorp and :kats regarding this issue:

snorp [4:23 PM]
well it's mostly video/webgl
honestly mostly video because there isn't much webgl around
@kats we can't filter the scrollbar items out on the WR side?
with some kind of composite flag

kats [4:24 PM]
so you'd just get a hole where the video would be?

snorp [4:24 PM]
right

kats [4:25 PM]
not on the WR side, no. not without serious surgery and i don't think anybody would want that kind of surgery in the WR code

snorp [4:25 PM]
bummer

kats [4:25 PM]
it's easier to do with the current compositor

snorp [4:26 PM]
we'd just skip a certain layer type?

kats [4:26 PM]
yeah, the scrollbar layers have a flag on them if they're activated
if they're inactive then you can't skip them

snorp [4:26 PM]
is it because WR just gets "this is a rounded filled rect"?

kats [4:27 PM]
i think it might actually be a blob image but basically yeah WR can't distinguish between it and other similar objects

snorp [4:27 PM]
I see

kats [4:27 PM]
we'd have to add a flag on the item which would bloat the entire display list

snorp [4:27 PM]
I agree that's bad
hmmm, well what to do
I don't think this is that big of an issue
and I'd rather have scrollbars than no videos

kats [4:27 PM]
yeah agreed

snorp [4:28 PM]
soooo

kats [4:28 PM]
might be worth asking the video people if there's a way to screenshot them

snorp [4:28 PM]
why don't we just table it for now? :slightly_smiling_face:
there is
but you need a compositor
and it's a giant PITA, etc

kats [4:28 PM]
is that how they do it on desktop?

snorp [4:28 PM]
desktop probably doesn't have anything crappy like SurfaceTexture

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Emily Toop (:fluffyemily) from comment #2)

In discussion with :snorp we decided that the presence of scrollbars is preferable to the presence of black holes in screenshots. It is recommended that users of the screenshot API either a) delay the taking of screenshots slightly until the scrollbars have disappeared from the screen, or crop the screenhots to remove the right and bottom after the image has been returned from the API.

Emily, should we resolve this bug as WONTFIX? Or fix the bug for the non-WebRender case? The Graphics team might ship Android WebRender later this year, but they haven't committed to a schedule yet.

Comment 4

[Emily Toop (:fluffyemily)]

Resolve as WONTFIX. It's not possible to fix this for non webrender either.

Comment 5

[Chris Peterson [:cpeterson]]

Thanks. I'll resolve this bug as WONTFIX and pass your screenshot suggestions on to the Fenix team.