Closed Bug 1178575 Opened 5 years ago Closed 5 years ago

In certain cases, box-shadow is drawn twice

Categories

(Core :: Web Painting, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: me, Assigned: mats)

References

Details

(Keywords: regression, testcase)

Attachments

(5 files)

In a certain fairly obscure (but realistic) set of circumstances, an element’s box-shadow is drawn twice, which for a semi-transparent box-shadow has the effect of making it twice as opaque as it should be.

The issue appears to require these conditions:

- The element must be contained in something with `overflow: hidden`;
- The element must have `position: absolute` and it must span the element with `overflow: hidden` (the easiest way is typically `left: 0; right: 0`);
- The element’s `height` CSS property must be changed from something small to something large (the precise magnitudes of the small and large seem to vary on the box-shadow spread radius);
- The element must have a semi-transparent box-shadow with no blur but with spread;

This happened to me in real and fairly complex code that I was writing, but it reduced nicely to the attached test case.

Reproduced on both Arch Linux (64-bit) and Mac OS X.
nsDisplayBoxShadowOuter::Paint calls ComputeDisjointRectangles, which produces two disjoint rectangles.  This code comes from https://hg.mozilla.org/mozilla-central/rev/1a0f2d658a49 .

(Prior to https://hg.mozilla.org/mozilla-central/rev/fdbe3aa72cdb this did a clip.)

Then it uses each as the aDirtyRect to a call to nsCSSRendering::PaintBoxShadowOuter, which in turn calls nsContextBoxBlur::BlurRectangle, which in the non-blur codepath does a FillRect.  This path completely ignores aDirtyRect.

The actually regression, though, came in this range (from Linux nightlies):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4b6d63b05a0a&tochange=2f8af55d6e9a
but I'm not sure why.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(mats)
Version: 38 Branch → Trunk
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #1)
> nsDisplayBoxShadowOuter::Paint calls ComputeDisjointRectangles, which
> produces two disjoint rectangles.  This code comes from
> https://hg.mozilla.org/mozilla-central/rev/1a0f2d658a49 .
> 
> (Prior to https://hg.mozilla.org/mozilla-central/rev/fdbe3aa72cdb this did a
> clip.)

To be clear:  it looks like these two revisions are, when combined, incorrect, although I don't know why the bug didn't show up until a week after the second one.
FWIW, it seems the bug only occurs when overflow occurs on both
the left and right sides, not when overflowing on just one side.
Attached patch 1008301-backoutSplinter Review
(In reply to David Baron [:dbaron] ⏰UTC-7 (busy Aug. 8-Aug. 30) from comment #1)
> (Prior to fdbe3aa72cdb this did a clip.)

The Clip call was moved inside PaintBoxShadowOuter instead, but you're right
that the aDirtyRect part fell out.

> The actually regression, though, came in this range (from Linux nightlies):
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=4b6d63b05a0a&tochange=2f8af55d6e9a
> but I'm not sure why.

Backing out bug 1008301 in a trunk build makes the problem go away.
Here's a backout patch for doing that if anyone wants to investigate why.
Flags: needinfo?(mats)
I was hoping you'd be able to fix this.
Flags: needinfo?(mats)
Attached patch fixSplinter Review
Like dbaron, I don't understand why the bug only manifests after the changes
in bug 1008301.  The fix seems simple enough though.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bb1770827e5
(looks green to me)
Assignee: nobody → mats
Attachment #8644519 - Flags: review?(roc)
Attached patch testsSplinter Review
Flags: needinfo?(mats)
Blocks: 613659
Keywords: regression, testcase
ComputeDisjointRectangles says it's only "approximate", so I wonder if this bug
can still occur if it generates two rectangles that overlap?
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/786625a84d87
https://hg.mozilla.org/mozilla-central/rev/927e481324f9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.