Closed Bug 693938 Opened 9 years ago Closed 9 years ago

Remote viewport ThebesBuffer always using CONTENT_ALPHA format

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: romaxa, Assigned: romaxa)

References

(Blocks 1 open bug)

Details

(Whiteboard: [MemShrink])

Attachments

(2 files, 4 obsolete files)

Found that all fennec remote offscreen viewports creating thebesLayer with CONTENT_ALPHA format....

not sure if it is regression, or it is done on purpose, but why do we need ALPHA channel for those thebes layers? with that we are using 2x more memory for each tab remote frame offscreen thebes layer
In ContainerState::PopThebesLayerData
transparentRegion.IsEmpty() - return false, that is why it is not opaque...
not sure if that is related to checkerboard, or bad clipping of remote viewport.
I set it force to true (for big layer size), and it seems to work fine, checkerboard visible, and no other artifacts...

roc do you know what is going on here?
Actually this problem making viewport rotation/updates much slower, because it is forced to be all 32bpp operations. more memory usage on GPU side, and memory bandwidth for composite operations.
I don't know. FrameLayerBuilder looks at the content of the ThebesLayer to determine whether it's transparent or not. The content will be transparent unless we can find a solid opaque background covering the layer.
hmm in the same place we do call FindOpaqueBackgroundColorFor with 0 index for that layer, and that function just return 0,0,0,0 color, because index is not valid
Root Document child list definitely have non-opaque areas, and there is no background which is covering that layer.
Can we just make root scroll frame opaque by default?
Also it make sense to switch checker-board Image layer to Opaque  mode
Attachment #566612 - Flags: feedback?(roc)
The checkerboard image part of your patch is correct.

Also, GetBackgroundImage should be changed to use RGB24 instead of ARGB32 as the format, since the checkerboard image is opaque. That could make compositing it slightly faster.

If there's no background covering the root scrollable layer, why should it be opaque?
Adding to not forget conclusions
Assignee: nobody → romaxa
Attachment #566612 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #566612 - Flags: feedback?(roc)
Attachment #567020 - Flags: review?(roc)
Attachment #567021 - Flags: review?(roc)
Comment on attachment 567020 [details] [diff] [review]
Make background checkerboard opaque

Review of attachment 567020 [details] [diff] [review]:
-----------------------------------------------------------------

The rest is good.

::: layout/ipc/RenderFrameParent.cpp
@@ +442,3 @@
>          }
>          else {
> +          data[y * BOARDSIZE + x] = 0xDDDDDD;

Don't do this, I think you should continue to set the alpha component to 0xFF.
Attachment #567020 - Flags: review?(roc) → review+
Comment on attachment 567021 [details] [diff] [review]
Mark PuppetWidget root scroll layer as opaque

Review of attachment 567021 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.cpp
@@ +623,5 @@
> +  if (FrameMetrics::ROOT_SCROLL_ID == id &&
> +      usingDisplayport &&
> +      !(root->GetContentFlags() & Layer::CONTENT_OPAQUE)) {
> +    // See bug 693938, attachment 567017 [details]
> +    NS_WARNING("Root scroll layer hasn't been marked as opaque, let's mark it so");

It's not the root scroll layer ... it's actually the root layer.

I would probably remove the ROOT_SCROLL_ID check and just say that we don't support transparent content with displayports, so if there's a displayport, we force the content to be opaque.
Set Alpha back
Attachment #567020 - Attachment is obsolete: true
Attachment #567021 - Attachment is obsolete: true
Attachment #567021 - Flags: review?(roc)
Attachment #567396 - Flags: review?(roc)
Keywords: checkin-needed
Comment on attachment 567394 [details] [diff] [review]
Make background checkerboard opaque

It is obsolete now after bug 694706 fixed
Attachment #567394 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/969648d51825
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink]
This fix reducing memory usage by Fennec scrollable offscreen thebes layer by 2 (10Mb->5Mb for active tab)
You need to log in before you can comment on or make changes to this bug.