Disable e10s when hardware acceleration is disabled

RESOLVED FIXED in mozilla35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cpeterson, Assigned: jimm)

Tracking

unspecified
mozilla35
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm2+)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

4 years ago
e10s is unhappy when hardware acceleration is disabled, e.g. bug 1026521 and bug 1026093. As a temporary workaround, we should add hardware acceleration to our list of conditions that can disable e10s during startup.
(Assignee)

Updated

4 years ago
Assignee: nobody → jmathies
(Assignee)

Comment 1

4 years ago
This is tricky, since we don't just check a pref for acceleration, we make a lot of last minute decisions in the widget code where we set up composition about turning acceleration on.

http://mxr.mozilla.org/mozilla-central/source/widget/xpwidgets/nsBaseWidget.cpp#771
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#3295
(Assignee)

Comment 2

4 years ago
Created attachment 8490769 [details] [diff] [review]
part 1 - win support

Bas, would you mind looking this over? I think I have all the checks I need based on a lot of poking around in widget/gfx code.
Attachment #8490769 - Flags: review?(bas)
(Assignee)

Updated

4 years ago
Blocks: 1068674
I got the sense that this was also (or primarily?) a Mac problem.
Comment on attachment 8490769 [details] [diff] [review]
part 1 - win support

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

This looks fine, I wonder if there isn't an easier way to detect this, but probably not as early on as this.
Attachment #8490769 - Flags: review?(bas) → review+
(Assignee)

Comment 5

4 years ago
(In reply to Bas Schouten (:bas.schouten) from comment #4)
> Comment on attachment 8490769 [details] [diff] [review]
> patch v.1
> 
> Review of attachment 8490769 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks fine, I wonder if there isn't an easier way to detect this, but
> probably not as early on as this.

Yeah nothing is initialized this early, I was hoping something like gfxPlatform would be but no luck.

(In reply to Bill McCloskey (:billm) from comment #3)
> I got the sense that this was also (or primarily?) a Mac problem.

Ah, thought mac and linux were ok. Working on a mac patch currently on top of this.
(Reporter)

Comment 6

4 years ago
(In reply to Jim Mathies [:jimm] from comment #5)
> (In reply to Bill McCloskey (:billm) from comment #3)
> > I got the sense that this was also (or primarily?) a Mac problem.
> 
> Ah, thought mac and linux were ok. Working on a mac patch currently on top
> of this.

Mac is affected. See bug 1026093.
(Assignee)

Comment 7

4 years ago
Created attachment 8490838 [details] [diff] [review]
part 2 - add macos support
(Assignee)

Updated

4 years ago
Attachment #8490769 - Attachment description: patch v.1 → part 1 - win support
(Assignee)

Updated

4 years ago
Attachment #8490838 - Flags: review?(jmuizelaar)
(Assignee)

Comment 8

4 years ago
Created attachment 8490840 [details] [diff] [review]
part 1 - win support

- merged to mc tip
Attachment #8490769 - Attachment is obsolete: true
Attachment #8490840 - Flags: review+
Comment on attachment 8490838 [details] [diff] [review]
part 2 - add macos support

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4568,5 @@
> +    SInt32 bugfix = nsCocoaFeatures::OSXVersionBugFix();
> +    if (major == 10 && minor == 6 && bugfix <= 2) {
> +      accelDisabled = true;
> +    }
> +#endif

This should be moved into a common helper so we don't accidentally have these two checks diverge.
Attachment #8490838 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 10

4 years ago
Created attachment 8491058 [details] [diff] [review]
macos support
Attachment #8490838 - Attachment is obsolete: true
Comment on attachment 8491058 [details] [diff] [review]
macos support

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4561,5 @@
>        }
>      }
> +
> +#if defined(XP_MACOSX)
> +    // See nsBaseWidget::ComputeShouldAccelerate

This comment can go.
Attachment #8491058 - Flags: review+
(Assignee)

Comment 12

4 years ago
Created attachment 8491064 [details] [diff] [review]
part 2 - macos support

I moved it into nsCocoaFeatures.
Attachment #8491058 - Attachment is obsolete: true
Attachment #8491064 - Flags: review?(jmuizelaar)
Comment on attachment 8491064 [details] [diff] [review]
part 2 - macos support

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4561,5 @@
>        }
>      }
> +
> +#if defined(XP_MACOSX)
> +    // See nsBaseWidget::ComputeShouldAccelerate

This comment can go.
Attachment #8491064 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 15

4 years ago
Created attachment 8491078 [details] [diff] [review]
rollup
Attachment #8490840 - Attachment is obsolete: true
Attachment #8491064 - Attachment is obsolete: true
Attachment #8491078 - Flags: review+
(Assignee)

Comment 17

4 years ago
https://hg.mozilla.org/mozilla-central/rev/04458f023e47
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.