1029919 - [OMTC][HWA off]<button:hover> outside parent's border-radius obtains a background/border in parent's inset box-shadow's color

Status: VERIFIED FIXED

(Core :: Graphics: Layers, defect)

Description

[j.j.]

Mozilla/5.0 (Windows NT 5.1; rv:33.0) Gecko/20100101 Firefox/33.0

I see this regression on XP in current Nightly, works in Fx30:

See testcase (reduced from a real-world-website),
hovering over <button>s draws a border in the color of the parent's inset box-shadow.
Actually, it's just a background-color, as you can see if button's opacity < 1.

Comment 1

[j.j.]

Attachment: testcase

Comment 2

[j.j.]

Attachment: testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Need a regression range.  Doesn't seem to be reproducible on Mac.  :(

Comment 4

[Alice0775 White]

Could you attach screenshot(actual and expected) of the problem?

Comment 5

[j.j.]

Attachment: screenshot after :hover over buttons

Button's background gets green (and stays green) onmouseover.
The second button is opaque, so you see a green border only.

This is on XP and of course it differs depending on OS specific button styling.

Expected: Display native buttons (no green background-color or border) after hovering over it.

Comment 6

[j.j.]

Attachment: screenshot from real-world

Screenshot from real website:
Translucent Button should not get red background after hovering.

Comment 7

[Alice0775 White]

I can reproduce the problem in Aurora32.0a2 and Nightly33.0a1 WindowsXP SP3 default visual style(i.e. Luna).
However, I cannnot reproduce in Beta31.0b4.


Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb8fc1380b00
Mozilla/5.0 (Windows NT 5.1; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140519202628
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/46d9ffb97fe3
Mozilla/5.0 (Windows NT 5.1; rv:32.0) Gecko/20100101 Firefox/32.0 ID:20140519224628
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bb8fc1380b00&tochange=46d9ffb97fe3
Triggered by:
46d9ffb97fe3	Bas Schouten — Bug 899785: Switch on OMTC and async video on windows. r=BenWa


Setting layers.offmainthreadcomposition.enabled = false fixes the problem

Comment 8

[Alice0775 White]

I can reproduce on Windows8.1update1, Windows7 if OMTC on and HWA off.

Graphics
Adapter Description	ATI Radeon HD 4300/4500 Series
Adapter Drivers	aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64
Adapter RAM	512
ClearType Parameters	Gamma: 2200 Pixel Structure: R ClearType Level: 50 Enhanced Contrast: 200
Device ID	0x954f
DirectWrite Enabled	false (6.2.9200.16492)
Driver Date	4-29-2013
Driver Version	8.970.100.1100
GPU #2 Active	false
GPU Accelerated Windows	0/1 Basic (OMTC)
Vendor ID	0x1002
WebGL Renderer	Google Inc. -- ANGLE (ATI Radeon HD 4300/4500 Series Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote	true
AzureCanvasBackend	skia
AzureContentBackend	cairo
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0

Comment 9

[Boris Zbarsky [:bzbarsky]]

Bas, can you take a look please?

Comment 10

[Bas Schouten (:bas.schouten)]

So after a long investigation, I've found some interesting information. This appears to be a bug in cairo clipping, it appears that the clip at nsCSSRendering.cpp:1526 fails to be applied properly if there's already a clip present for the small rectangle surrounding the button (which would be disjoint with this clip) (i.e. if I remove that clip the button always gets a green background, if I leave it in it only has a green background when doing a partial invalidation of the button). I know we've had clipping bugs with Cairo in the past, Jeff, does this sort of issue ring a bell for you?

Comment 11

[Bas Schouten (:bas.schouten)]

Attachment: Patch that 'fixes' the bug

This patch fixes the bug, providing evidence of the rootcause suggested.

Comment 12

[Bas Schouten (:bas.schouten)]

Attachment: Set the proper extent on the clip_path in cairo

So as far as my investigation has been able to show this bug is indirectly the results of the extents on a clip path being incorrectly set. At cairo-clip.c:276 we set the extents on the clip_path struct, but those extents no longer contain the real extents but rather the intersection of these clips.

This patch fixes that, however it does have the downside of extents of the top level clip no longer being the intersection of all the clips on the stack, I think that's okay and will only make a difference in some rare perf cases though.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8447783 [details] [diff] [review]
Set the proper extent on the clip_path in cairo

Review of attachment 8447783 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure this is correct. I don't see why the extents that are set need to be extents of the path and can't be the extents of the intersection.

Comment 14

[Bas Schouten (:bas.schouten)]

Attachment: Properly composite when using an unbounded operator even if the extents don't intersect

I believe this is the correct fix this time, I'd like to hear what you think :-).

Comment 15

[Bas Schouten (:bas.schouten)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e8fac17305a

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7e8fac17305a

Comment 17

[j.j.]

Mozilla/5.0 (Windows NT 5.1; rv:33.0) Gecko/20100101 Firefox/33.0

Verified fixed in today's Nightly