Transparent windows drawn to by the GL compositor render incorrectly

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: acomminos, Assigned: acomminos)

Tracking

50 Branch
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(2 attachments)

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.
(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.
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.
Attachment #8772490 - Flags: review?(nical.bugzilla) → review+
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 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.
Attachment #8772489 - Flags: review?(jgilbert) → review+
Attachment #8772489 - Flags: review?(jmuizelaar)
Comment on attachment 8772489 [details]
Bug 1286847 - Remove calls to XGetGeometry from the compositor thread.

https://reviewboard.mozilla.org/r/65286/#review63630
Attachment #8772489 - Flags: review?(jmuizelaar) → review+
Pushed by acomminos@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/0c8cfad5d7a4
https://hg.mozilla.org/mozilla-central/rev/0de8a950f4be
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.