regression in 0.12, forces use of dedicated GPU on macbook

[tomfogg]

version 0.12 appears to have a regression of #894 which allowed apps using glutin to use the integrated GPU on macbooks. 0.12 appears to force the dedicated GPU on all the time

[mkeeler]

I thought it may have been related to turning on hardware acceleration in src/platform/macos/helpers.rs from this PR so I tested with this extra fn locally

diff --git a/src/lib.rs b/src/lib.rs
index d2dff34..5a4a093 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -294,6 +294,16 @@ impl<'a> ContextBuilder<'a> {
         self.pf_reqs.srgb = srgb_enabled;
         self
     }
+
+
+    /// Sets whether hardware acceleration should be enabled, disabled or whether the runtime can choose
+    ///
+    /// The default value is `Some(true)` which will force hardware acceleration
+    #[inline]
+    pub fn with_hw_accel(mut self, hw_accel: Option<bool>) -> Self {
+        self.pf_reqs.hardware_accelerated = hw_accel;
+        self
+    }
 }

 impl GlWindow {

Then I compiled Alacritty with this local version of glutin and with this change:

diff --git a/src/window.rs b/src/window.rs
index 42a1785..07eef70 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -190,7 +190,8 @@ fn create_gl_window(
 ) -> ::std::result::Result<glutin::GlWindow, glutin::CreationError> {
     let context = ContextBuilder::new()
         .with_srgb(srgb)
-        .with_vsync(true);
+        .with_vsync(true)
+	.with_hw_accel(None);
     ::glutin::GlWindow::new(window, context, event_loop)
 }

No luck though. Even with not explicitly enabling hardware acceleration it still required the high perf gpu.

[tywtyw2002]

It seems like the PR: #963 broken the integrated graphics switching.

In this PR, file helpers.rs:41 force add the attribute NSOpenGLPFAAccelerated and with this attribute, the system force uses the dedicated GPU.

It needs to add the if-else sentences to switch the attribute between NSOpenGLPFAAccelerated and NSOpenGLPFAAllowOfflineRenderers to allow the integrated GPU to be selected.

(As the image shown, the alacritty no longer as the dependency of dedicated GPU )

@kvark @tomfogg

[mkeeler]

@tywtyw2002 What was the code change you were testing there? Just changing the NSOpenGLPFAAccelerated to NSOpenGLPFAAllowOfflineRenderers?

[tywtyw2002]

Yes. That just a hack.

Someone needs to fix this code.

[zeroDivisible]

@tywtyw2002 I'm totally new to Rust but I tried (I think) recompilling Alacritty with local glutin and changes of NSOpenGLPFAAccelerated to NSOpenGLPFAAllowOfflineRenderers. This was from latest master of both and still made Alacritty require dedicated GPU. Just wondering, do you maybe have any tips how I can hack my way for this not to be case?

My main goal is to get Alacritty to work locally (hacking if needed) without burning the laptop in the process :)

[francesca64]

It's tough for me to work on this, since my Mac build doesn't have a discrete GPU, but this seems to be quite informative: https://bugs.chromium.org/p/chromium/issues/detail?id=88788

[mkeeler]

Was there a reason why determining the GL version from app kit version was changed to initializing pixel formats.

This diff works to get rid of the high perf gpu requirement:

diff --git a/src/platform/macos/helpers.rs b/src/platform/macos/helpers.rs
index a41161f..d060243 100644
--- a/src/platform/macos/helpers.rs
+++ b/src/platform/macos/helpers.rs
@@ -35,26 +35,16 @@ pub fn get_gl_profile<T>(
             Ok(NSOpenGLProfileVersion4_1Core)
         }
     } else if let GlRequest::Latest = opengl.version {
-        // now, find the latest supported version automatically
-        let mut attributes = vec![
-            // Note: we assume here that non-accelerated contexts don't care
-            NSOpenGLPFAAccelerated as u32,
-            NSOpenGLPFAOpenGLProfile as u32,
-            0, // this is where we put the test profile
-            0,
-        ];
-        for &profile in &[NSOpenGLProfileVersion4_1Core, NSOpenGLProfileVersion3_2Core] {
-            attributes[2] = profile as u32;
-            let id = unsafe {
-                NSOpenGLPixelFormat::alloc(nil).initWithAttributes_(&attributes)
-            };
-            if id != nil {
-                unsafe { msg_send![id, release] }
-                return Ok(profile);
-            }
-        }
-        // nothing else to do
-        Ok(NSOpenGLProfileVersionLegacy)
+       let app_kit_version = unsafe { NSAppKitVersionNumber.floor() };
+
+       if app_kit_version >= NSAppKitVersionNumber10_9 {
+               Ok(NSOpenGLProfileVersion4_1Core)
+       } else if app_kit_version >= NSAppKitVersionNumber10_7 {
+
+               Ok(NSOpenGLProfileVersion3_2Core)
+       } else {
+               Ok(NSOpenGLProfileVersionLegacy)
+       }
     } else {
         Err(CreationError::OpenGlVersionNotSupported)
     }

The one less than desirable aspect is having to have an unsafe block for accessing the NSAppKitVersionNumber but other than that this is basically the code as it existed before for determining the latest opengl version.

[francesca64]

The motive for the change is explained in #963, which fixed #781.

If you use GlRequest::Specific to bypass this codepath, do things work as expected?

[mkeeler]

@francesca64 Modifying alacritty to create the context with a 4.1 version also works:

diff --git a/src/window.rs b/src/window.rs
index 987332e..3753fbb 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -189,6 +189,7 @@ fn create_gl_window(
     srgb: bool,
 ) -> ::std::result::Result<glutin::GlWindow, glutin::CreationError> {
     let context = ContextBuilder::new()
+       .with_gl(glutin::GlRequest::Specific(glutin::Api::OpenGl, (4,1)))
         .with_srgb(srgb)
         .with_vsync(true);
     ::glutin::GlWindow::new(window, context, event_loop)

Looking back at #963 it seems like the main issue was that build_nsattributes wasn't ever adding the NSOpenGLPFAAccelerated attribute. The only reason for modifying it to use initWithAttributes instead of using the app kit version was "When Latest non-compatibility version is requested, try 4.1 and 3.2 explicitly instead of relying on software versions." It seems only tangentially related to that PR.

Another thing to consider is that the initWithAttributes method is only available in the macOS 10.10 SDK and above so really this branch could just return NSOpenGLProfileVersion4_1Core and be done as this shouldn't properly link pre 10.10

[mkeeler]

So macOS API docs can be tricky. Looking further initWithAttributes is available pre 10.10 but its behavior changed a little.

[mkeeler]

@francesca64 Ignore me. I think the reasoning is that while macOS 10.9 has support for opengl 4.1 maybe not all the hardware in older macs that support 10.9 are compatible?

[mkeeler]

So I think swapping offline renderers for accelerated does work (not sure why it didn't for me before). Additionally you can specify offline renderers and accelerated and it will still work.

diff --git a/src/platform/macos/helpers.rs b/src/platform/macos/helpers.rs
index a41161f..bb30598 100644
--- a/src/platform/macos/helpers.rs
+++ b/src/platform/macos/helpers.rs
@@ -37,14 +37,14 @@ pub fn get_gl_profile<T>(
     } else if let GlRequest::Latest = opengl.version {
         // now, find the latest supported version automatically
         let mut attributes = vec![
-            // Note: we assume here that non-accelerated contexts don't care
+            NSOpenGLPFAAllowOfflineRenderers as u32,
             NSOpenGLPFAAccelerated as u32,
             NSOpenGLPFAOpenGLProfile as u32,
             0, // this is where we put the test profile
             0,
         ];
         for &profile in &[NSOpenGLProfileVersion4_1Core, NSOpenGLProfileVersion3_2Core] {
-            attributes[2] = profile as u32;
+            attributes[3] = profile as u32;
             let id = unsafe {
                 NSOpenGLPixelFormat::alloc(nil).initWithAttributes_(&attributes)
             };

The way I understand offline renderers is that its to switch between and use arbitrary gpus not connected to a display. In the case of laptops they are integrated and discrete but in other cases like a mac pro they are both equally powered discrete gpus. Is there any reason why we cant just always call initWithAttributes here with offline enabled? I am not sure if its a general fix but it definitely seems to fix alacritty which is what I personally want.

[zeroDivisible]

@mkeeler I don't understand the specifics of what you've written (as I lack low level knowledge in the area), but I can confirm that your latest diff indeed makes Alacritty stick with integrated graphics card.

[francesca64]

Does this change make it impossible to use the discrete GPU with glutin, though?

[mkeeler]

That is the question. Is there a program known to use glutin and require the discrete gpu I could test out with the changes. I do not believe allowing offline renderers conflicts with hw accel but without testing it would be hard to know for certain

[francesca64]

The gfx examples are something you'd likely want to run on the discrete GPU.

[tywtyw2002]

@zeroDivisible
You also need to remove all NSOpenGLPFAAccelerated and NSOpenGLPFADoubleBuffer.

@mkeeler

https://gist.github.com/tywtyw2002/c65c5fe7a7a86fef2dcd65a2bd06e19b

[tomfogg]

I haven't tried the proposed changes but bear in mind the original change also needed NSSupportsAutomaticGraphicsSwitching to be added to the mac app bundle's Info.plist to stop the discrete GPU from activating. If this is the same with these changes then apps would still use the dGPU by default unless they specifically added this flag.

[mkeeler]

I think that my PR leaves things in a good state. The get_gl_profile now respects double buffering and hw accel from the pf_reqs and the ContextBuilder now has functions to configure those as well so applications like alacritty can override the defaults. Like I mentioned in the PR, having NSOpenGlPFAAllowOfflineRenderers always on does not prevent switching over to the discrete gpu. How macOS makes the determination I am not sure but the gfx-rs examples still required the discrete GPU and alacritty didn't.

[mkeeler]

@francesca64 Is there a date for the next glutin release when I can expect to see these changes on crates.io?

[francesca64]

Wednesday seems likely.