Closed Bug 945088 Opened 8 years ago Closed 8 years ago

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


(Core :: Graphics, defect)

Not set



Tracking Status
firefox26 --- unaffected
firefox27 + fixed
firefox28 + fixed
firefox29 + fixed


(Reporter: roc, Assigned: nrc)



(Whiteboard: [qa-])


(1 file)

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

Probably up Nick's alley.
This is so egregious it must be a regression. I hope it's not mine!
Hi Roc, Can you give us a little more detail on the impact to our users?
Flags: needinfo?(roc)
I expect the rendering of some sites to be broken.
Flags: needinfo?(roc)
Assignee: nobody → ncameron
Depends on: 942672
(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.
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.
And yes, it is a regression - works on v26 (beta), fails on v27 (aurora).
Last good revision: f97307cb4c95 (2013-09-23)
First bad revision: 1fda74e33e06 (2013-09-24)

Last good revision: f97307cb4c95
First bad revision: 70221b582ca1

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.
Depends on: 916034
Attachment #8344467 - Flags: review?(matt.woodrow)
Attachment #8344467 - Flags: review?(matt.woodrow) → review+
(Does this fix need to go on any branches?)
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
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.
Attachment #8344467 - Flags: approval-mozilla-beta?
Attachment #8344467 - Flags: approval-mozilla-aurora?
(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.
Attachment #8344467 - Flags: approval-mozilla-beta?
Attachment #8344467 - Flags: approval-mozilla-beta+
Attachment #8344467 - Flags: approval-mozilla-aurora?
Attachment #8344467 - Flags: approval-mozilla-aurora+
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #18)
> Bug 942672 isn't on Beta (Fx27), so I left the reftest part of the patch out.

Perfect, thanks!
Flags: in-testsuite+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.