Closed Bug 1249312 Opened 4 years ago Closed 4 years ago

Ensure Accelerated Canvas on Mac happens only on 10.7 or later

Categories

(Core :: Graphics, defect, major)

44 Branch
x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox44 --- wontfix
firefox45 + fixed
firefox46 + fixed
firefox47 + fixed

People

(Reporter: mchang, Assigned: mchang)

References

Details

(Whiteboard: gfx-noted,regression)

Attachments

(1 file)

Due to bug 1244569 and 1246633, it seems like accelerated skia canvas breaks some things. From the daily today, we decided to disable accelerated skia canvas on OS X versions < 10.8
Blocks: 1246633, 1244569
No longer depends on: 1246633
From https://dxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPlatformMac.cpp#433, we should not be enabling accelerated skia canvas on less than 10.7, but from bug 1244569, the user is reporting 10.6.8. Maybe we're not respecting accelerated canvas prefs somewhere, digging.
In other words, I have a feeling that UseAcceleratedCanvas and UseAcceleratedSkiaCanvas should not both exist.
Come to think of it, I don't know what was happening here: https://dxr.mozilla.org/mozilla-central/source/gfx/layers/composite/LayerManagerComposite.cpp#1470 - were we forcing accelerated CG somehow?
Comment on attachment 8720899 [details]
MozReview Request: Bug 1249312: UseAcceleratedCanvas on Mac should really be UseAcceleratedSkiaCanvas. r?mchang

https://reviewboard.mozilla.org/r/35503/#review32189
Attachment #8720899 - Flags: review?(mchang) → review+
Summary: Disable accelerated Canvas on OS X versions < 10.8 → Disable accelerated Skia Canvas on OS X versions < 10.8, and accelerated CG canvas always
We're still unsure if the google maps problem in bug 1246633 is strictly related to 10.7 as someone reproduced it on 10.10.
Summary: Disable accelerated Skia Canvas on OS X versions < 10.8, and accelerated CG canvas always → Ensure Accelerated Canvas on Mac happens only on 10.7 or later
Duplicate of this bug: 1244569
Blocks: 1249659
https://hg.mozilla.org/mozilla-central/rev/7595a1641deb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment on attachment 8720899 [details]
MozReview Request: Bug 1249312: UseAcceleratedCanvas on Mac should really be UseAcceleratedSkiaCanvas. r?mchang

Approval Request Comment
[Feature/regressing bug #]: bug 1150944, accelerated canvas on OS X
[User impact if declined]: Graphic artifacts such as bug 1244569 
[Describe test coverage new/current, TreeHerder]: Manual, treeherder
[Risks and why]: Low - Accelerated canvas was only supposed to be enabled on 10.7 or higher, but there was a bug in the check where lower versions such as 10.6 would actually enable accelerated canvas.
[String/UUID change made/needed]: None
Attachment #8720899 - Flags: approval-mozilla-beta?
Attachment #8720899 - Flags: approval-mozilla-aurora?
Tracking since this is a fairly recent regression (shipped with 42)
Comment on attachment 8720899 [details]
MozReview Request: Bug 1249312: UseAcceleratedCanvas on Mac should really be UseAcceleratedSkiaCanvas. r?mchang

Should prevent some issues on macs, please uplift to aurora
Attachment #8720899 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Milan, do you confirm that we really want that in 45 & 45esr or this can ride the train? This is really late in the cycle.
thanks
Flags: needinfo?(milan)
I appreciate the lateness of the request.  About 24% of our OS X user base is affected, and has had sub par canvas experience since this regression slipped in.

Before we do though - Mason, we would want bug 1249659 as well, right?  This only deals with 10.6 systems, and it's bug 1249659 that also handles it for 10.7 ones.
Flags: needinfo?(milan) → needinfo?(mchang)
(In reply to Milan Sreckovic [:milan] from comment #17)
> I appreciate the lateness of the request.  About 24% of our OS X user base
> is affected, and has had sub par canvas experience since this regression
> slipped in.
> 
> Before we do though - Mason, we would want bug 1249659 as well, right?  This
> only deals with 10.6 systems, and it's bug 1249659 that also handles it for
> 10.7 ones.

Correct, we probably want both this and bug 1249659. I was going to ask for beta uplift on bug 1249659  after this one lands.
Flags: needinfo?(mchang)
Comment on attachment 8720899 [details]
MozReview Request: Bug 1249312: UseAcceleratedCanvas on Mac should really be UseAcceleratedSkiaCanvas. r?mchang

I am very unhappy to see this kind of uplift request that late in the cycle.
Next time, please be on top of your features, Bug 1244569 has been reported a month ago :/
Taking it for the only reason that 45 is an ESR.
Attachment #8720899 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.