Closed Bug 1027380 Opened 6 years ago Closed 5 years ago

Paint flashing from an inactive layer manager saturates the transparent layers

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
Using paint flashing in some cases where I have a transparent layer that uses an inactive layer manager to draw sometimes yields an overly saturated paint-flash color making the background even more difficult to solve. We shouldn't paint-flash inactive layer manager because the active layer manager will flash it.
Attachment #8442467 - Flags: review?(matt.woodrow)
Attachment #8442467 - Flags: review?(matt.woodrow) → review+
Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f57cf85fd128

Didn't know we had tests for this. Nice :D
CC'ing mstange since matt thinks you might of tried to land something like this before.
Attached patch patch (obsolete) — Splinter Review
This gives us paint flashing for offscreen rendering, which the failing test require.

Or maybe we should just remove that test. Up to you.
Assignee: nobody → bgirard
Attachment #8442467 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8442944 - Flags: review?(matt.woodrow)
Your patch looks much better than mine in bug 1009680 and it won't cause the test failure that I encountered.
Oh, I see now that that was exactly the intention.
Right, we both had the same v1.
Attachment #8442944 - Flags: review?(matt.woodrow) → review+
-  if (presContext->GetPaintFlashing()) {
+  if (presContext->GetPaintFlashing() &&
+    !aLayer->Manager()->IsInactiveLayerManager()) {
     ^^ wrong indent

(In reply to Benoit Girard (:BenWa) (Off until June 30th) from comment #4)
> Or maybe we should just remove that test. Up to you.

We could also create a reftest instead. For example we could copy layout/reftests/reftest-sanity/invalidation.html and use it with

pref(nglayout.debug.paint_flashing,true) != invalidation.html about:blank

Since reftests should always be using widget layers, this should work with the v1 patch. We can also make it conditional on layersGPUAccelerated if it doesn't.
Duplicate of this bug: 1009680
Opps. I though I had landed that.

Yes we could create a new better test for it. We need to be careful how we compare because if we only paint flash for thebes layer and about:blank can use a color layer.
Attached patch patch rebasedSplinter Review
Attachment #8442944 - Attachment is obsolete: true
ttps://hg.mozilla.org/integration/mozilla-inbound/rev/24b61c6aad57
https://hg.mozilla.org/mozilla-central/rev/24b61c6aad57
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.