Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow alacritty to run on the integrated graphics card on macOS #210

Closed
uvesten opened this issue Jan 7, 2017 · 30 comments
Closed

Allow alacritty to run on the integrated graphics card on macOS #210

uvesten opened this issue Jan 7, 2017 · 30 comments

Comments

@uvesten
Copy link
Contributor

uvesten commented Jan 7, 2017

As it is now, alacritty triggers a switch to the discrete graphics card on dual-graphics macbook pro's. It feels like a bit of overkill to run down the laptop battery by using the discrete card just to run a terminal.

Apple's official doc for running openGL contexts on the integrated card is at https://developer.apple.com/library/content/qa/qa1734/_index.html , no idea how it relates to rust, though.

@jwilm
Copy link
Contributor

jwilm commented Jan 7, 2017

This is a great recommendation; thanks!

@jwilm jwilm added this to the Version 1.0 milestone Jan 7, 2017
@jwilm
Copy link
Contributor

jwilm commented Jan 7, 2017

It looks like this isn't anything we do in code. Instead, the configuration is in an Info.plist.

This can be solved when we start distributing an Alacritty.app.

@tomfogg
Copy link

tomfogg commented Jan 9, 2017

Just tried making an .app bundle with NSSupportsAutomaticGraphicsSwitching set to true in the Info.plist. It appears this is not enough for it to use the integrated GPU.

It looks like NSOpenGLPFAAllowOfflineRenderers needs to be set on the NSOpenGLPixelFormatAttribute list too as per this note

I'm not sure where to do that in the rust code

@tomfogg
Copy link

tomfogg commented Jun 6, 2017

Dug into this a bit more and managed to get it not to activate the DGPU by changing this in the glutin package. I noticed you're using a forked version, could this or an option to enable this be added to your fork?

diff --git a/src/api/cocoa/helpers.rs b/src/api/cocoa/helpers.rs
index 7738e54..e132eb3 100644
--- a/src/api/cocoa/helpers.rs
+++ b/src/api/cocoa/helpers.rs
@@ -55,6 +55,7 @@ pub fn build_nsattributes<T>(pf_reqs: &PixelFormatRequirements, opengl: &GlAttri
         NSOpenGLPFAAlphaSize as u32, alpha_depth as u32,
         NSOpenGLPFADepthSize as u32, pf_reqs.depth_bits.unwrap_or(24) as u32,
         NSOpenGLPFAStencilSize as u32, pf_reqs.stencil_bits.unwrap_or(8) as u32,
+        NSOpenGLPFAAllowOfflineRenderers as u32, 
         NSOpenGLPFAOpenGLProfile as u32, profile,
     ];

@jwilm
Copy link
Contributor

jwilm commented Jun 6, 2017

Hey @tomfogg,

Nice work chasing down a solution!

I'm currently trying to get off the Glutin fork. It would probably make sense to contribute such a patch upstream (probably winit for cocoa api).

@tomfogg
Copy link

tomfogg commented Jun 6, 2017

Thanks for the response, I've added a pull request for glutin to add the flag

@tomfogg
Copy link

tomfogg commented Jul 7, 2017

That change on glutin has been merged now so this issue should get fixed when alacritty switches to the mainline glutin

@jwilm
Copy link
Contributor

jwilm commented Jul 7, 2017

cc #234

@jwilm
Copy link
Contributor

jwilm commented Jul 20, 2017

We're now on latest Glutin/winit. Is there anything else Alacritty needs to do to support this?

@jwilm jwilm removed the A - deps label Jul 20, 2017
@tomfogg
Copy link

tomfogg commented Jul 20, 2017

The app bundle made with make app uses the NSSupportsAutomaticGraphicsSwitching flag so that should be it. Just tried with the latest master and its all working as expected so I reckon you can close this.

@jwilm
Copy link
Contributor

jwilm commented Jul 20, 2017

That's great! Thanks for the quick follow-up.

@jwilm jwilm closed this as completed Jul 20, 2017
@tomfogg
Copy link

tomfogg commented Jan 10, 2018

There seems to be a regression on this in commit b83a26f The update to glutin 0.12 makes the dedicated GPU come on all the time

@lompabo
Copy link

lompabo commented Mar 6, 2018

I can confirm the issue is still there as of today

@relapi
Copy link

relapi commented Mar 15, 2018

@jwilm Would be wonderful if this could get fixed, I've had to switch to using iterm2 when on battery :(

@oschrenk
Copy link

oschrenk commented Apr 6, 2018

Is rust-windowing/glutin#980 the right issue to track upstream to wait for the regression fix?

@gmile
Copy link

gmile commented Apr 18, 2018

Seeing similar issue with Alacritty. Was coding during a conference, and noticed battery go down super quickly. macOS reported Alacritty draining battery the most. I related this to the Alacritty using the beefier GPU, but not sure how to confirm this.

alacritty --version reports 0.1.0 with no git revision. Best I have is that it was installed from source on march 13.

@gmile
Copy link

gmile commented Apr 18, 2018

Re-buliding from 20aa45c (parent of b83a26f) fixed the problem for me. macOS doesn't recognize Alacritty as battery eater anymore.

@kylehendricks
Copy link

Confirmed that building master will force use of the discrete graphics. Since this issue was about initially adding support for using integrated graphics (which was added), should we close this and open a new issue since this is now a regression?

@mkeeler
Copy link
Contributor

mkeeler commented May 1, 2018

@kylehendricks There was a new issue for the regression in #1211 but @jwilm closed as a duplicate of this one. So it would seem this issue is the one to track the regression.

@jwilm
Copy link
Contributor

jwilm commented May 5, 2018

@oschrenk yes, once rust-windowing/glutin#980 is resolved, we should be able to close this again.

@mkeeler
Copy link
Contributor

mkeeler commented Jun 28, 2018

rust-windowing/glutin#980 is closed and incorporated into the glutin 0.17 release that hit crates.io yesterday.

I was going to work on getting the necessary changes to alacritty incorporated but its going to take a bit longer as a new winit version was also pulled in which has plenty of breaking changes that prevent alacritty from compiling.

@chrisduerr
Copy link
Member

Yeah there are going to be quite a few changes necessary. I was going to get to that this weekend, but if anyone else wants to give it a shot, feel free!

@mkeeler
Copy link
Contributor

mkeeler commented Jun 28, 2018

I have got a branch going in my personal fork if anyone wanted a look: https://github.com/mkeeler/alacritty/tree/glutin-0.17-upgrade

The two biggest changes not pushed up right now are going to be that winit now seems to use f64s instead of f32s for most of the scaling factor stuff. I guess alacritty could upgrade to f64 in all the places where its used too but these look like they will be extensive changes.

Secondly where we provided pixel positions in x,y coordinates before there is now a LogicalPosition struct to hold these. There is also a LogicalSize struct which will need to be incorporated. All of this seems to be related to better HiDPI support.

@chrisduerr
Copy link
Member

Yeah I expect the LogicalPosition changes to introduce a bit of work, because right now calculations like number of cells in each row directly depend on actual window size.

@mkeeler
Copy link
Contributor

mkeeler commented Jun 28, 2018

Breaking: The entire API for sizes, positions, etc. has changed. In the majority of cases, winit produces and consumes positions and sizes as LogicalPosition and LogicalSize, respectively. The notable exception is MonitorId methods, which deal in PhysicalPosition and PhysicalSize. See the documentation for specifics and explanations of the types. Additionally, winit automatically conserves logical size when the DPI factor changes.

That gives me some hope that it might just be a matter of converting (i32, i32) into a LogicalPosition everywhere as it makes it sound like before most things that were using x,y before were logical coordinates anyways.

@chrisduerr
Copy link
Member

@mkeeler Hey, I've seen that you've made another commit to that branch 22 hours ago. Would you like to finish and make a PR for this? I'm always happy about new contributors. :)

If you have any questions I'd love to help out either on github or irc (freenode/#alacritty) of course. :)

@mkeeler
Copy link
Contributor

mkeeler commented Jun 30, 2018

@chrisduerr Just got it compiling. I am going to test things out a bit locally and if all goes well open a PR here shortly.

@mkeeler
Copy link
Contributor

mkeeler commented Jun 30, 2018

Well my branch compiles but doesn't work quite right. Only a quarter of the terminals window gets used so something with all the sizing updates isn't quite right.

@mkeeler
Copy link
Contributor

mkeeler commented Jun 30, 2018

@chrisduerr I opened a work-in-progress PR, its still not working totally but if you could add some comments or give some pointers about where to look to figure out my issue with text output only appearing in the lower left quadrant that would be much appreciated.

@jonleopard
Copy link

Hello everyone,

So I'm not sure if I'll be of much help here (I know very little about Rust), but I can definitely confirm that opening Alacritty.app triggers the high performance gpu. Here's a video capture. If you take a look at the middle column on the bottom, you'll see the moment it switches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests