945088 - Reftest layout/reftests/bugs/942672-1.html fails on Windows non-D2D

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=08fd80f4b2bf

This looks like a genuine, bad bug related to native theme drawing.

Probably up Nick's alley.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is so egregious it must be a regression. I hope it's not mine!

Comment 2

[Benjamin Kerensa [:bkerensa]]

Hi Roc, Can you give us a little more detail on the impact to our users?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I expect the rendering of some sites to be broken.

Comment 4

[Nick Cameron [:nrc]]

(notes to self)

I started to investigate this. It is easy to repro locally. I can't actually understand why we don't see this bug more often.

Initial observations - it is not the case that only the test is bad and the ref is ok. Both are rendered incorrectly. It seems that the rect we render is translated incorrectly in both cases. In the test case, the clip appears wrong, but that is because it is correct and the image is in the wrong place. In the ref case the clip is in the same (incorrect) place as the image. (I say clip rect here, I'm not suggesting that that is the cause, just using it as a way to describe the appearance).

In both cases, when you mouse over the button (which highlights it), it is rendered correctly. Similarly, removing the opacity from the style fixes things.

Comment 5

[Nick Cameron [:nrc]]

I don't think this is an alpha recovery problem, because we are not going down that drawing path. We go down the RENDER_STATE_NATIVE_DRAWING paths for both correct and incorrect rendering. The difference comes down to the translation which comes from the transform on the gfxContext. I am not sure why that changes, where it is set, or how it causes the error though.

Comment 6

[Nick Cameron [:nrc]]

And yes, it is a regression - works on v26 (beta), fails on v27 (aurora).

Comment 7

[Nick Cameron [:nrc]]

Last good revision: f97307cb4c95 (2013-09-23)
First bad revision: 1fda74e33e06 (2013-09-24)

Pushlog:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f97307cb4c95&tochange=1fda74e33e06

Comment 8

[Nick Cameron [:nrc]]

Last good revision: f97307cb4c95
First bad revision: 70221b582ca1

Pushlog:

http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f97307cb4c95&tochange=70221b582ca1

Comment 9

[Nick Cameron [:nrc]]

Regressed by the change to gfxContext.cpp in attachment 804327 [details] [diff] [review], bug 916034. I'm going to have to understand why that change was necessary before I can fix this.

Comment 10

[Nick Cameron [:nrc]]

Attachment: Use the correct device offset in gfxContext

Comment 11

[Nick Cameron [:nrc]]

try push: https://tbpl.mozilla.org/?tree=Try&rev=77a32f246bf8

Comment 12

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0ca7cc8ef55

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Excellent, thanks.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(Does this fix need to go on any branches?)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b0ca7cc8ef55

Comment 16

[Nick Cameron [:nrc]]

Comment on attachment 8344467 [details] [diff] [review]
Use the correct device offset in gfxContext

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 916034
User impact if declined: bad rendering of non-opaque native widgets on windows
Testing completed (on m-c, etc.): m-c, just missed uplift
Risk to taking this patch (and alternatives if risky): low, but could possible affect other, unknown rendering paths
String or IDL/UUID changes made by this patch: none

Note: I would like to uplift the whole path to Aurora, but only the code change (not the test change) to Beta, because the broken test does not exist for Beta.

Comment 17

[Nick Cameron [:nrc]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> (Does this fix need to go on any branches?)

Yes (just nom'ed), I was waiting to see if the fix would make the uplift, but it seems not.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6e64771384ca
https://hg.mozilla.org/releases/mozilla-beta/rev/e01187ca1702

Bug 942672 isn't on Beta (Fx27), so I left the reftest part of the patch out.

Comment 19

[Nick Cameron [:nrc]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #18)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/6e64771384ca
> https://hg.mozilla.org/releases/mozilla-beta/rev/e01187ca1702
> 
> Bug 942672 isn't on Beta (Fx27), so I left the reftest part of the patch out.

Perfect, thanks!