1286847 - Transparent windows drawn to by the GL compositor render incorrectly

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Andrew Comminos [:acomminos]]

Currently, using the GL compositor with the patches from bug 1283299 and ARGB windows force-enabled, we draw the shadow with the default blend mode (causing stacking shadows). We should consider either

1. Performing a glClear on the back buffer before or after each frame
2. Switch to using SOURCE blending for the root layer

Option 1 seems the least painless, and would let us get rid of a round-trip to the X server to fetch the window geometry to remove artifacts when the back buffer gets resized during a composite.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

(In reply to Andrew Comminos [:acomminos] from comment #0)
> Currently, using the GL compositor with the patches from bug 1283299 and
> ARGB windows force-enabled, we draw the shadow with the default blend mode
> (causing stacking shadows). We should consider either
> 
> 1. Performing a glClear on the back buffer before or after each frame
> 2. Switch to using SOURCE blending for the root layer

Yes. 1 seems reasonable. Doing it before seems most reasonable.

Comment 2

[Andrew Comminos [:acomminos]]

Attachment: Bug 1286847 - Remove calls to XGetGeometry from the compositor thread.

Review commit: https://reviewboard.mozilla.org/r/65286/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65286/

Comment 3

[Andrew Comminos [:acomminos]]

Attachment: Bug 1286847 - Only enable scissor testing where required in the GL compositor.

Review commit: https://reviewboard.mozilla.org/r/65288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65288/

Comment 4

[Andrew Comminos [:acomminos]]

These patches disable the scissor test outside of where we immediately need it, and remove the GLContext target size infrastructure for fetching the size of an X window. The latter was intended to prevent drawing garbage, which won't occur when we have a queued unbounded clear.

Comment 5

[Nicolas Silva [:nical]]

Comment on attachment 8772490 [details]
Bug 1286847 - Only enable scissor testing where required in the GL compositor.

https://reviewboard.mozilla.org/r/65288/#review62512

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8772489 [details]
Bug 1286847 - Remove calls to XGetGeometry from the compositor thread.

https://reviewboard.mozilla.org/r/65286/#review63322

::: gfx/layers/opengl/CompositorOGL.cpp:710
(Diff revision 1)
> -  if (viewportSize != mWidgetSize.ToUnknownSize()) {
> -    mGLContext->fScissor(0, 0, viewportSize.width, viewportSize.height);
> -  }
> -
>    RefPtr<CompositingRenderTargetOGL> rt =
> -    CompositingRenderTargetOGL::RenderTargetForWindow(this, viewportSize);
> +    CompositingRenderTargetOGL::RenderTargetForWindow(this,

I can't review CompositorOGL.cpp.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8772489 [details]
Bug 1286847 - Remove calls to XGetGeometry from the compositor thread.

https://reviewboard.mozilla.org/r/65286/#review63630

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c8cfad5d7a4
Remove calls to XGetGeometry from the compositor thread. r=jgilbert,jrmuizel
https://hg.mozilla.org/integration/autoland/rev/0de8a950f4be
Only enable scissor testing where required in the GL compositor. r=nical

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0c8cfad5d7a4
https://hg.mozilla.org/mozilla-central/rev/0de8a950f4be