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

wrong screen resolution on macos sonoma with oclp #196

Closed
1 task done
Un1q32 opened this issue Oct 6, 2023 · 15 comments
Closed
1 task done

wrong screen resolution on macos sonoma with oclp #196

Un1q32 opened this issue Oct 6, 2023 · 15 comments

Comments

@Un1q32
Copy link
Contributor

Un1q32 commented Oct 6, 2023

Describe the bug

on my mid 2014 mbp running macos 14.0 sonoma using opencore legacy patcher, the screen resolution neofetch reports is 1280x800, but the correct screen resolution is 2560x1600

  • Does this issue still occur in the master branch? (Required)

Expected behavior

neofetch should report a screen resolution of 2560x1600

Screenshots

Screenshot 2023-10-06 at 2 17 44 AM

Config file

(its the default config)
conf.txt

Verbose log

neofetch.log

@catumin
Copy link
Collaborator

catumin commented Oct 10, 2023

I'll try and look into this in the morning. My Macbook is 2015, so it should have the same behavior.

@hykilpikonna
Copy link
Owner

I just upgraded to Sonoma today, but unfortunately I could not replicate the issue. Maybe it's because of the igpu... can you show me your system_profiler SPDisplaysDataType -xml?

image image

@catumin
Copy link
Collaborator

catumin commented Oct 10, 2023

image image

system_profiler SPDisplaysDataType -xml

I have the same incorrect behavior on my MacBook. iGPU could make sense. My guesses would be that or Retina display.

@hykilpikonna
Copy link
Owner

Hmm... so the resolution in terms of "screen real-estate" is actually 1280x800 (like the amount of 12-point font text you can fit on the screen is the same as on a 1280x800 screen). But it's scaled up 2x using HiDPI for the 2560x1600 display.

But it's also not like regular HiDPI where e.g. it renders a 5120x2880 virtual screen then downscale it for my 2560x1440 display. In this case, neofetch should definitely show the 2560 pixel resolution instead of the 5120 virtual rendering resolution.

Should I add a special case for detecting and showing pixel resolution if it's a retina display? Or should I keep everything the same and always show the actual "screen real-estate" resolution for all situation? (Maybe I can add something at the end showing it's a retina display)

image

@Un1q32
Copy link
Contributor Author

Un1q32 commented Oct 10, 2023

my system_profiler SPDisplaysDataType -xml

out.txt

@catumin
Copy link
Collaborator

catumin commented Oct 10, 2023

I could be wrong, but I think this block:

https://github.com/hykilpikonna/hyfetch/blob/master/neofetch#L3865-L3870

Is supposed to add the scaling factor to the output of the resolution. But instead currently doesn't actually do anything.

@Un1q32
Copy link
Contributor Author

Un1q32 commented Oct 11, 2023

What is the PlistBuddy command? I don't have that on my system
edit: oh its in /usr/libexec

CarterLi added a commit to CarterLi/neofetch that referenced this issue Oct 11, 2023
1. fix detection when `screenresolution` is not available
2. make scale factor detection actually work
3. remove `screenresolution` dependency

EDIT: applied changes suggested by @hykilpikonna and fixed a
[bug](hykilpikonna/hyfetch#196) that neofetch
fails to print scale factors when `system_profiler` fails to detect
refresh rates.
@CarterLi
Copy link
Contributor

CarterLi commented Oct 11, 2023

Since this code was written by me, I guess I should do some after-sale services.

The file attached above shows that system_profiler SPDisplaysDataType -xml fails to detect refresh rate

<key>_spdisplays_resolution</key>
<string>1280 x 800</string>

While on my MBP, it shows

<key>_spdisplays_resolution</key>
<string>1728 x 1117 @ 120.00Hz</string>

This line

[[ $scale_factor -gt 1 ]] && spdisplays_resolution="${spdisplays_resolution// @/ @${scale_factor}x @}"

assumes spdisplays_resolution contains an @ character and tries to append scale factor after it, but it fails to do so.

Fixed in my original PR

@catumin
Copy link
Collaborator

catumin commented Oct 11, 2023

Should I add a special case for detecting and showing pixel resolution if it's a retina display? Or should I keep everything the same and always show the actual "screen real-estate" resolution for all situation? (Maybe I can add something at the end showing it's a retina display)

I think the best solution would be to keep the output as "screen real-estate", with a note afterwards of it being Retina. The logic seems to be mostly there for it, but just needs to be fixed up. Maybe like:

Resolution: XxY (Retina 2x)

Or something like that.

@hykilpikonna
Copy link
Owner

Fixed in my original PR

Thanks for the fix!!

@CarterLi
Copy link
Contributor

CarterLi commented Oct 11, 2023

Before starting a discussion, we need to define what is a Resolution that neofetch detects

There are 4 resolutions:

  1. The native (highest) resolution supported by hardware monitor.
  2. The highest usable resolution. This resolution is limited by hardware monitor, the cable, the port and GPU. For example if you use a HDMI 1.2 cable to connect a 4K monitor, you can't choose 4K resolution output because 4K resolution is only supported by HDMI 1.4 and later.
  3. The output resolution ( the image resolution generated by GPU ). It will be scaled to the native resolution by monitor.
  4. The scaled resolution. It will be scaled to the output resolution by software (if the app you are using supports HIDPI) or system (if the app you are using doesn't support HIDPI)
  • Resolution 1 is the value reported in About My Mac image. It can only be detected by parsing EDID on most systems.
  • Resolution 2 is the maximum resolution you can choose in the system settings in Windows and Linux.
    image
  • Resolution 3 is the value of _spdisplays_pixels. This is the resolution you chose in Windows and Linux control panel.
  • Resolution 4 is the value of _spdisplays_resolution. This is the resolution you chose in macOS System Settings.
    image
  • Scale factor, as shown in the screenshot of Linux (gnome), is Resolution3 / Resolution4. On macOS it's currently always 2 for retina display. It's also known as device pixel radio

My PR uses Resolution4 @ ScaleFactor, which also matches the behavior of original neofetch with screenresolution installed.

@catumin
Copy link
Collaborator

catumin commented Oct 11, 2023

For reference of current Retina output with the fix included:
image

I think this output is the "most correct", personally. I can definitely see many expecting the output to match what we would get from _spdisplays_pixels though.

@CarterLi
Copy link
Contributor

CarterLi commented Oct 11, 2023

FYI, resolution detection in neofetch is messy.

image

@CarterLi
Copy link
Contributor

Off topic. Neofetch's Media player detection is broken in macOS too. It runs iTunes / Apple Music and prints Unknown Artist - Unknown Album - Unknown Song. That's all.

CarterLi added a commit to CarterLi/neofetch that referenced this issue Oct 12, 2023
Format `${scaled_width}x${scaled_height} @${scale_factor}x @ ${refresh_rate} Hz` is used to match the result format of macOS

Ref: hykilpikonna/hyfetch#196 (comment)
CarterLi added a commit to CarterLi/neofetch that referenced this issue Oct 12, 2023
Format `${scaled_width}x${scaled_height} @${scale_factor}x @ ${refresh_rate} Hz`
is used to match the format of macOS

Note: current implementation requires `wayland-info`, which is usually
provided by package
[`wayland-utils`](https://gitlab.freedesktop.org/wayland/wayland-utils)

Ref: hykilpikonna/hyfetch#196 (comment)
CarterLi added a commit to CarterLi/neofetch that referenced this issue Oct 12, 2023
Format `${scaled_width}x${scaled_height} @${scale_factor}x @ ${refresh_rate} Hz`
is used to match the format of macOS

Note: current implementation requires `wayland-info`, which is usually
provided by package
[`wayland-utils`](https://gitlab.freedesktop.org/wayland/wayland-utils)

Ref: hykilpikonna/hyfetch#196 (comment)
@Un1q32
Copy link
Contributor Author

Un1q32 commented Oct 19, 2023

I would still prefer it show the real screen resolution like it does without oclp, but I guess this good enough

@Un1q32 Un1q32 closed this as completed Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants