Transparent windows drawn to by the GL compositor render incorrectly

RESOLVED FIXED in Firefox 50

Status

()

Core
Graphics: Layers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: acomminos, Assigned: acomminos)

Tracking

50 Branch
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
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.
(Assignee)

Comment 2

2 years ago
Created attachment 8772489 [details]
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/
Attachment #8772489 - Flags: review?(jgilbert)
Attachment #8772490 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 3

2 years ago
Created attachment 8772490 [details]
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/
(Assignee)

Comment 4

2 years ago
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.

Updated

2 years ago
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+

Comment 8

2 years ago
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

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0c8cfad5d7a4
https://hg.mozilla.org/mozilla-central/rev/0de8a950f4be
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.