Remove the MOZ_WIDGET_GONK build flag for OverlayImage implementation

NEW
Unassigned

Status

()

Core
Graphics: Layers
3 years ago
3 years ago

People

(Reporter: shelly, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
This new type of image might be applied to other platform as well, and since the implementation of OVerlayImage is not gonk-dependent, we might want to remove the build flag of MOZ_WIDGET_GONK.
(Reporter)

Comment 1

3 years ago
Created attachment 8517200 [details] [diff] [review]
Remove the gonk build flag for OverlayImage implementation
(Reporter)

Updated

3 years ago
Flags: needinfo?(chung)
The patch looks good for me, I am still debugging why it not work on desktop.
Flags: needinfo?(chung)
Created attachment 8518675 [details] [diff] [review]
WIP

This patch fixes the missing part of your oringinal patch.

While debugging, I found it works on B2G by some maybe problematic behaviour:
[1] is always run on B2G, but not desktop. This maybe a layer recycling bug on B2G. The overlay image patch creates a new ImageClient in ImageContainer, which is attached to the ImageLayer in [2], so we should handle such change.

By the way, this may still not puncture hole on some platform, since this patch and original overlay image patch works only for CompositorOGL, if you want to it works on linux, you will need fix BasicCompositor.

So I think we may need a good reason to change correspond part on different compositor.

@shelly
BTW, can you add test case after your TV media stream part complete?

[1] http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/ClientImageLayer.cpp?from=ClientImageLayer.cpp#155
[2] http://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/ImageClient.cpp?from=ImageClient.cpp&case=true#425
(Reporter)

Comment 4

3 years ago
Hi Marco, according to Chiajung's comment 3, it seems that removing the build flag of b2g only isn't a doddle, how do you think about that?
(Reporter)

Comment 5

3 years ago
Hi Chiajung, after discussing with Marco, we will keep the OverlayImage as b2g only for now, but since it looks like the current implementation for b2g is base on a problematic bug, we will still leave it as a follow-up issue.
Blocks: 1026358
I think it is okay to leave as a follow up of the maybe problematic behavior. I think we should also follow up any further requirement about it in this bug. :)
You need to log in before you can comment on or make changes to this bug.