1195209 - "Get Screenshot" through webIDE does not work on landscape mode

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

When trying to use "get screenshot" on WebIDE you just get a blank grey screen instead of the actual contents of the screen.

Comment 1

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I mean Monitor -> Screenshot, to be clear.

Comment 2

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Attachment: example output

Comment 3

[Josh Cheng [:josh]]

Hi Evelyn, Hi Rex,
Could you help to check on this? Thanks!

Comment 4

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

This is probably something @evelyn and @rexboy can help if TV side send an all-grey image to WebIDE. For screenshot we use an off-screen canvas to capture the entire screen. It's more likely a graphics issue to me, @daosheng can provide more help on it.

Comment 5

[Evelyn Hung]

Yep, agree SC's comment 4. I'm not sure how WebIDE get a screenshot, but I assume it use the same API as Gaia system uses. Here is the API implementation in Gecko:

https://dxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#1122

Daosheng, could you investigate it a bit? If there is anything you need from Gaia side, please ask Rex. Thanks!

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Here is the code of getting screenshot from WebIDE:
https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/device.js?offset=0#43

Comment 7

[Daosheng Mu[:daoshengmu]]

Ok! I will investigate it and answer it shortly.

Comment 8

[KM Lee [:rexboy] (inactivate after Sep. 22)]

As SC and Evelyn said, I guess this should be handled by just Gecko API called directly from WebIDE without Gaia's involve. If you found anything need Gaia's help please just NI me again.

Comment 9

[Daosheng Mu[:daoshengmu]]

I have a quick investigation on my aries. Its screenshot is a gray image as well. I think it could be a underlying bug. I will continue to spend time in finding the reason.

Comment 10

[Daosheng Mu[:daoshengmu]]

Attachment: In portrait mode, the screenshot is good.

Comment 11

[Daosheng Mu[:daoshengmu]]

Attachment: In landscape mode, the screenshot is gray one.

Comment 12

[Daosheng Mu[:daoshengmu]]

I have reproduced it on my FxOS devices include nexus-5 and TV. This bug is only happened on landscape mode for screenshot. On TV, it is always on landscape mode. Therefore, I think that is the reason why it is so easy to be caught. Currently, I think it might due to nsIPresShell::RenderDocument. (https://dxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp#4810).

Comment 13

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I'm doing some snapshot related work. If this is not a urgent bug, I can take a look next week.

Comment 14

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #13)
> I'm doing some snapshot related work. If this is not a urgent bug, I can
> take a look next week.

Thanks Jerry.

Comment 15

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Please note that this bug is also found on stingray TV which is based on b2g v2.1, which means this functionality is broken for a while (or never works perhaps).

Comment 16

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: rotation.png

We would like to use a) instead of b) for landscape mode.

Comment 17

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: modify the matrix calculation for landscape mode. v1

Please check attachment 8653612 [details].

Comment 18

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Please land the attachment 8653613 [details] [diff] [review] into m-c.
Just a trivial fix, so I don't create a try.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/461098c64803

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/461098c64803

Comment 21

[Josh Cheng [:josh]]

Hi David,
Do we need this for 2.2r?

Comment 22

[David Flanagan [:djf]]

Josh: yes, it sounds like this would be useful to have working correctly in 2.2r. Flip phones won't have a landscape mode, but sliders will. Thanks for asking.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/c90978c0c488

Comment 24

[Daisuke]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #15)
> Please note that this bug is also found on stingray TV which is based on b2g
> v2.1, which means this functionality is broken for a while (or never works
> perhaps).

I applied patch[1] to stingray TV (based on v2.1), but still gray image is available on WebIDE screenshot.
Additional patches are necessary?


FYI
 This screenshot never works on stingray, but layerscope[2] works well.

[1]https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/c90978c0c488
[2]https://wiki.mozilla.org/Platform/GFX/LayerScope

Comment 25

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Daisuke from comment #24)
> (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment
> #15)
> > Please note that this bug is also found on stingray TV which is based on b2g
> > v2.1, which means this functionality is broken for a while (or never works
> > perhaps).
> 
> I applied patch[1] to stingray TV (based on v2.1), but still gray image is
> available on WebIDE screenshot.
> Additional patches are necessary?
> 
> 
> FYI
>  This screenshot never works on stingray, but layerscope[2] works well.
> 
> [1]https://hg.mozilla.org/releases/mozilla-b2g37_v2_2r/rev/c90978c0c488
> [2]https://wiki.mozilla.org/Platform/GFX/LayerScope

Hi Kusaba-san,

I finally got v2.1 running on Flame today. The snapshot issue is verified fixed on my flame with latest v2.1 gecko + patch on this bug. You may need to do further investigation on your side and Jerry will provide more debugging information. Hope this can help you lock down the issue quickly.

Comment 26

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Hi Daisuke,

Please make sure we got the correct snapshot result from compositor at [1]. Use the Map() and GetData() interface(like [2]) to dump the content to file. Then use a raw-image viewer to check it.

Finally, dump the drawTarget content at [3] through DrawTarget::Snapshot() and SourceSurface::GetDataSurface().

With these two checkpoints, we can make sure the error is at compositor or content side.

[1]
http://hg.mozilla.org/releases/mozilla-b2g34_v2_1/annotate/d1ed7de67f5a/gfx/layers/client/ClientLayerManager.cpp#l451

[2]
https://hg.mozilla.org/mozilla-central/annotate/dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/gfx/gl/GLReadTexImageHelper.cpp#l294

[3]
http://hg.mozilla.org/releases/mozilla-b2g34_v2_1/annotate/d1ed7de67f5a/gfx/layers/client/ClientLayerManager.cpp#l461

Comment 27

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: debug_webide_snapshot.diff

for debug webidl

Comment 28

[Taro Suzuki]

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #26)
> Hi Daisuke,
> 
> Please make sure we got the correct snapshot result from compositor at [1].
> Use the Map() and GetData() interface(like [2]) to dump the content to file.
> Then use a raw-image viewer to check it.
> 
> Finally, dump the drawTarget content at [3] through DrawTarget::Snapshot()
> and SourceSurface::GetDataSurface().
> 
> With these two checkpoints, we can make sure the error is at compositor or
> content side.
> 
> [1]
> http://hg.mozilla.org/releases/mozilla-b2g34_v2_1/annotate/d1ed7de67f5a/gfx/
> layers/client/ClientLayerManager.cpp#l451
> 
> [2]
> https://hg.mozilla.org/mozilla-central/annotate/
> dd2a1d737a64d9a3f23714ec5cc623ec8933b51f/gfx/gl/GLReadTexImageHelper.cpp#l294
> 
> [3]
> http://hg.mozilla.org/releases/mozilla-b2g34_v2_1/annotate/d1ed7de67f5a/gfx/
> layers/client/ClientLayerManager.cpp#l461

Hi Jerry-san.

Thanks for your patch.

I tried to get raw images and attached. it seems all "0" data.
Do you think that the error is at compositor side ?

Comment 29

[Taro Suzuki]

Attachment: raw images (TV)

raw images captured by TV environment.

Comment 30

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: dump_fb.patch

Please try this patch again.

It tries to use glReadPixel function to dump the fb to file.
If we still see the all zero file, the glReadPixel might not work. Then we need to check the opengl implementation.

Comment 31

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified as fixed on Flame master and Aries master. Screenshots taken using WebIDE works on landscape as well as portrait orientation.

Device: Aries 2.6 Master
BuildID: 20151112122058
Gaia: 27bc9412ca607648bc398b25bb1ae25653b2b278
Gecko: 3cc3b1968524248450c465c4ea2ee5596ffa65f2
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Flame 2.6 Master
BuildID: 20151112030244
Gaia: 27bc9412ca607648bc398b25bb1ae25653b2b278
Gecko: 3cc3b1968524248450c465c4ea2ee5596ffa65f2
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Note:
We don't have a 2.2r device so unable to verify that.

Comment 32

[Evelyn Hung]

I don't think we fix the issue *on TV* here. Let's reopen it.

Comment 33

[Evelyn Hung]

[Blocking Requested - why for this release]:

Hi Taro-san,
Per comment 30, do you have any finding?

Comment 34

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Making this bug "resolved" again since we do fix screenshot issue on phone, file Bug 1266625 for tracking TV specific issue.