Closed Bug 1273252 Opened 4 years ago Closed 4 years ago

WARP WebGL is not disabled with failIfMajorPerformanceCaveat

Categories

(Core :: Canvas: WebGL, defect, critical)

47 Branch
All
Windows
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox47 + fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: jrmuizel, Assigned: jgilbert)

References

Details

(Keywords: perf, Whiteboard: [gfx-noted])

Attachments

(2 files)

We should not ship WARP until this is fixed or we have the potential to make Google Maps bad for people.
Blocks: 1260944
[Tracking Requested - why for this release]: Ruining Google Maps performance would be bad.
Flags: needinfo?(jgilbert)
Keywords: perf
OS: Unspecified → Windows
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Version: unspecified → 47 Branch
Assignee: nobody → jgilbert
Flags: needinfo?(jgilbert)
Severity: normal → critical
From 1aea1bd168d6a61165a1038c7debfa1d940c9896 Mon Sep 17 00:00:00 2001
 failIfMajorPerformanceCaveat.
---
 dom/canvas/WebGLContext.cpp | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/54334/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54334/
Attachment #8755021 - Flags: review?(jmuizelaar)
Comment on attachment 8755021 [details]
MozReview Request: Bug 1273252 - r?jrmuizel - WARP should hit

https://reviewboard.mozilla.org/r/54334/#review51022
Attachment #8755021 - Flags: review?(jmuizelaar) → review+
Hi Jeff, is this a recent regression? Was it a problem in Fx46? I use google maps on my win10 machine and do not see a perf issue. I am a week away from RC so unless this is a critical recent regression, I really doubt we will be able to get this uplifted to Beta47. Thanks!
Flags: needinfo?(jmuizelaar)
(In reply to Ritu Kothari (:ritu) from comment #4)
> Hi Jeff, is this a recent regression? Was it a problem in Fx46? I use google
> maps on my win10 machine and do not see a perf issue. I am a week away from
> RC so unless this is a critical recent regression, I really doubt we will be
> able to get this uplifted to Beta47. Thanks!

This is for people who get software WebGL. You're probably always getting hardware WebGL, so this bug has no effect for you.
Unfortunately, we uplifted WARP fixes into 47, so we really should take this as well.
Flags: needinfo?(jmuizelaar)
Comment on attachment 8755021 [details]
MozReview Request: Bug 1273252 - r?jrmuizel - WARP should hit

Approval Request Comment
[Feature/regressing bug #]: 1260944
[User impact if declined]: Bad performance on Google Maps and similar sights
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low - this gets those users back to where they were before bug 1260944.
[String/UUID change made/needed]:
Attachment #8755021 - Flags: approval-mozilla-beta?
Comment on attachment 8755021 [details]
MozReview Request: Bug 1273252 - r?jrmuizel - WARP should hit

This improves performance on Google Maps (recent regression), Beta47+
Attachment #8755021 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hello Wes, could we get this patch uplifted to moz-beta, before I gtb 47.0b9 (Thursday)? Thanks!
Flags: needinfo?(wkocher)
I'm having trouble getting this patch to apply to any branch. Can you doublecheck that the patch still applies to beta?
Flags: needinfo?(wkocher) → needinfo?(jgilbert)
Attached patch beta patchSplinter Review
Flags: needinfo?(jgilbert)
Attachment #8756182 - Flags: review+
(In reply to Wes Kocher (:KWierso) from comment #9)
> I'm having trouble getting this patch to apply to any branch. Can you
> doublecheck that the patch still applies to beta?

This applies. I can try landing it tomorrow in case it doesn't build for some reason.
https://hg.mozilla.org/releases/mozilla-beta/rev/c4cf7fcd62d3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
It looks like this has not landed on central or aurora yet.
Status: RESOLVED → REOPENED
Flags: needinfo?(jgilbert)
Resolution: FIXED → ---
(In reply to Jeff Muizelaar [:jrmuizel] from comment #13)
> It looks like this has not landed on central or aurora yet.

Weird, yeah.
Flags: needinfo?(jgilbert)
Comment on attachment 8755021 [details]
MozReview Request: Bug 1273252 - r?jrmuizel - WARP should hit

Approval Request Comment
(see beta?/+ comments)
Attachment #8755021 - Flags: approval-mozilla-aurora?
Keywords: leave-open
Comment on attachment 8755021 [details]
MozReview Request: Bug 1273252 - r?jrmuizel - WARP should hit

Landed in beta, we want that in aurora too.
Attachment #8755021 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Target Milestone: mozilla47 → ---
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.