Macchina-CLI / libmacchina

A library providing access to all sorts of system information.
https://crates.io/crates/libmacchina
MIT License
68 stars 20 forks source link

[BUG] macOS resolution reports 0fps #119

Closed DannyJJK closed 2 years ago

DannyJJK commented 2 years ago

Describe the bug On macOS 12.5 (21G72) on a Macbook Pro 13-inch 2019, the resolution is reported as: 2880x1800@0fps (as 1440x900)

The resolution is correct but the fps is wrong, I don't think it should be 0.

To Reproduce Steps to reproduce the behavior:

  1. Use macOS (difficult if you don't own a Mac, sorry!)
  2. Install macchina v6.0.6
  3. Run macchina

Expected behavior I would expect it should be reported as something like: 2880x1800@60fps (as 1440x900)

(I'm not sure what the correct value is for this hardware.)

However if this is not supported on macOS, then it should be omitted like so: 2880x1800 (as 1440x900) And come up on the macchina --doctor output.

macchina --doctor output Let's check your system for errors... Here's a summary:

We've collected 18 readouts, including 3 failed read(s) and 1 read(s) which resulted in a warning. Readout "Packages" failed with message: No packages found — Do you have a package manager installed? Readout "LocalIP" failed with message: Unable to get local IPv4 address. Readout "Backlight" failed with message: This metric is not available on this platform or is not yet implemented by libmacchina.

1 of the 4 unsuccessful read(s) resulted in a warning: Readout "Distribution" threw a warning with message: Since you're on macOS, there is no distribution to be read from the system.

System Information You don't have to provide this information if you're not comfortable doing so, but it'll help us solve the issue a lot faster.

grtcdr commented 2 years ago

Hi,

It might be that we need to update some of our macOS dependencies, perhaps a change was introduced and we didn't have the time to pick it up.

However if this is not supported on macOS

This issue is new, as the framerate used to work fine in the past.

I'll see what I can do :)

grtcdr commented 2 years ago

Sorry for making you wait this long!

As I don't possess the hardware to verify the reproducibility of this issue, all we can do for the moment is wait and see if other users of macOS are experiencing the same problem.

DannyJJK commented 2 years ago

No worries. If there are any changes you think might solve the problem and you want to me to test a build, just let me know.

grtcdr commented 2 years ago

@123marvin123 Is this perhaps occurring on your machine too?

123marvin123 commented 2 years ago

Is this happening with an external monitor, or are you only connected to the internal display? The readout works fine for my M1 Macbook Air, as well as an external 4K monitor.

Can you send a screenshot of system information when going to "About this Mac -> System Report -> Displays (under Hardware)"? The menus might have different names, my macOS language is German.

DannyJJK commented 2 years ago

I'm only connected to the internal display. My macOS language is Spanish, not my first language but I'm learning it :) Here you go:

image
123marvin123 commented 2 years ago

Interesting, it looks like for some reason, the core graphics api will not return the refresh rate for some models. I guess that's a bug or at least inconsistency by Apple. Some other libraries also have to deal with this problem: https://github.com/glfw/glfw/commit/aab08712dd8142b642e2042e7b7ba563acd07a45

I just added another check in my latest commit: https://github.com/Macchina-CLI/libmacchina/commit/f3faad845d7f1f06dbc42ff86d73af6018dd1fed

Is it possible for you to try it out?

DannyJJK commented 2 years ago

@123marvin123 I've tried it using your commit as follows:

use libmacchina::GeneralReadout;

fn main() {
    use libmacchina::traits::GeneralReadout as _;

    let general_readout = GeneralReadout::new();

    println!("{:?}", general_readout.resolution());
}

outputs: 2880x1800@60fps (as 1440x900)

This appears to be fixed. Thanks so much!

123marvin123 commented 2 years ago

Nice, I'll merge the change then, and it will be present in the next update, I guess.