Closed Bug 1259541 Opened 4 years ago Closed 4 years ago

Avoid clearing backbuffer in nsBaseWidget::CreateBackBufferDrawTarget()

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jrmuizel, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 6 obsolete files)

Clearing currently shows up as around 10% when playing video.

We shouldn't need to clear our recycled back buffer. LayerManagerComposite::Render() should be clearing the necessary parts when it calls ClearRect for the rects of mRegionToClear.
Assignee: nobody → sotaro.ikeda.g
mRegionToClear seemed to be set only from nsWindow::UpdateThemeGeometries() via LayerManager::SetRegionToClear(). It seems necessary to update how to set mRegionToClear.
(In reply to Sotaro Ikeda [:sotaro] from comment #1)
> mRegionToClear seemed to be set only from nsWindow::UpdateThemeGeometries()
> via LayerManager::SetRegionToClear(). It seems necessary to update how to
> set mRegionToClear.

I believe the region from UpdateThemeGeometries is on the only part that needs clearing because it's the only part that we need transparency. I believe we draw opaque content over the rest of the window so we don't need to clear it even if are drawing to a B8G8R8A8 surface.
FWIW, I tried commenting out the ClearRect and didn't notice anything different other than the performance improvement. I did not, however, try pushing to try.
I also tried to commenting out the ClearRect on my laptop(Win8). It caused artifact around tabs. nsWindow::UpdateThemeGeometries() seems to care only button area.
Yeah, with Windows Aero Glass, then our layer tree will have real transparency that we need to clear.

We already have this code to tell the compositor that it can skip the clear when the layer tree is opaque:
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/LayerManagerComposite.cpp#903

We could extend that further to instead compute the transparent region of the window (by walking the layer tree and subtracting areas covered by CONTENT_OPAQUE layers) and pass that to the compositor instead of a bool. This should be super fast for platforms without glass, since the root layer will have CONTENT_OPAQUE.

We'd only need to clear the intersection of the transparent/invalid regions, which should be fairly fast.
mRegionToClear is a post-compositing effect.

On Windows, the control buttons (minimize, maximize, quit) are drawn by the window manager, but underneath our content. We need to ensure that the pixels that they are in are entirely clear so that they show through, without being obstructed by any of our layers.
Fix build failure on android and b2g.
Attachment #8736258 - Attachment is obsolete: true
attachment 8736293 [details] [diff] [review] does not handle a case that invalid region is extended by widget.
Fix build failure on gonk.
Attachment #8737733 - Attachment is obsolete: true
Attachment #8737734 - Flags: review?(matt.woodrow)
Comment on attachment 8737734 [details] [diff] [review]
patch - Reduce clearing backbuffer in nsBaseWidget::CreateBackBufferDrawTarget()

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

::: gfx/layers/basic/BasicCompositor.cpp
@@ +524,5 @@
>      return;
>    }
>  
> +  LayoutDeviceIntRect clearRect;
> +  if (!aOpaque) {

I think we can just get rid of aOpaque entirely, since aOpaqueRegion conveys the same information.
Attachment #8737734 - Flags: review?(matt.woodrow) → review+
Apply the comment. Carry "r=:mattwoodrow".
Attachment #8737734 - Attachment is obsolete: true
Attachment #8737999 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ab1e55d01de8
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.