994088 - BasicCompositor repaints everything on every frame

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Chris Lord [:cwiiis]]

BasicCompositor currently redraws everything on every frame to work around an unfiled bug (see bug #993639 comment #2). Seeing as this is apparently an e10s issue, let's at least special-case this to e10s.

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Only redraw everything with e10s

Note, I see errors with masks when doing partial updates - I don't know if this is an error in BasicCompositor or Moz2D yet. (or some third option)

Comment 2

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8404000 [details] [diff] [review]
Only redraw everything with e10s

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

::: gfx/layers/basic/BasicCompositor.cpp
@@ +413,5 @@
>    invalidRegionSafe.And(aInvalidRegion, intRect);
>  
> +  // FIXME: Redraw the whole screen in every frame to work around e10s
> +  //        repainting issues. See Bug 933639, comment #2.
> +  if (XRE_GetProcessType() != GeckoProcessType_Default) {

I don't think this works, the compositor always runs in the parent process.

Comment 3

[Matt Woodrow (:mattwoodrow)]

(In reply to Chris Lord [:cwiiis] from comment #1)
> Created attachment 8404000 [details] [diff] [review]
> Only redraw everything with e10s
> 
> Note, I see errors with masks when doing partial updates - I don't know if
> this is an error in BasicCompositor or Moz2D yet. (or some third option)

My guess would be LayerTreeInvalidation.cpp computing the wrong area for changes.

Comment 4

[Chris Lord [:cwiiis]]

Attachment: Don't redraw everything on every frame

I think you committed a patch that fixed the issue of tabs not redrawing? If not, how's that going? Would it be a good idea to just let that error go in the hope that more testing will result in better STR and an easier path to fix the bug?

Comment 5

[Matt Woodrow (:mattwoodrow)]

I didn't land, no. Apparently it didn't work.

Comment 6

[Matt Woodrow (:mattwoodrow)]

I've followed up again in bug 994546. Let's figure out what the situation is exactly before landing this.

Comment 7

[Chris Lord [:cwiiis]]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c666f8f24d29

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c666f8f24d29