Fix display_name returning index instead of name on macOS - #147
ralphweng2023 wants to merge 1 commit into
Conversation
Root cause: SDL_GetDisplayName returns display index ('0', '1', etc.)
instead of actual display names on newer macOS versions.
Solution: Use native macOS NSScreen.localizedName API via Objective-C++
to get proper display names like 'Built-in Retina Display'.
Changes:
- core_driver.cpp: Add macOS-specific code path using sk_macos_get_display_name()
- core_driver_macos.mm: New Objective-C++ file implementing NSScreen API
- CMakeLists.txt: Add macOS .mm files to build
Tested on macOS - now correctly returns 'Built-in Retina Display'
instead of '0'.
connorclancydev
left a comment
There was a problem hiding this comment.
General Information
This is pull is a fix for mac that lets mac correctly find the display name instead of integers
Code Quality
Code is well commented allowing people who are maintaining the code to understand what is done by the code, additionally as far as I can tell tha changes made successfully only target mac users not windows or linux. The code should be able to be changed easily in the future should it need to be as it is well commented and clear to read.
Functionality
When Tested on my Mac it seemed like the tests didnt fail I followed the instrutions for testing as written in the pull request and there were not any issues with the code returning the index instead of the name so I would say its functional
Testing
unit tests were not modified or made more of all the tests do seem to work with this change if more tests need to be added for this it can likely be added in a windows unit test or some other graphics testing motion, but this PR does fix the bug for the moment
Pull Request Details
The Pull request is successful in describing the changes and what to test so that people may replicate results to ensure that the change fixes the issue. The PR does successfully solve the issue which should assist MACOs Users. More effort might need to be done down the line to ensure longevity for MACos if updates they make deteriorate functions further but for the moment this PR is all good
Description
This PR fixes the
display_name()function returning display index numbers ("0", "1", "2") instead of actual display names on macOS.Problem
On newer macOS versions,
SDL_GetDisplayName()returns the display index as a string instead of the actual display name. This is a known SDL2 limitation.Before:
After:
Root Cause
SDL_GetDisplayName()in SDL2 does not properly retrieve display names on macOS 10.15+ (Catalina and later).Solution
Use native macOS
NSScreen.localizedNameAPI via Objective-C++ to get proper display names. The implementation:core_driver_macos.mmfile with macOS-specific codeNSScreen.localizedNamewhich returns proper names like "Built-in Retina Display" or "DELL U2720Q"SDL_GetDisplayName()on older macOS versions (pre-10.15) or other platformsFiles Changed
core_driver.cpp: Added macOS-specific code path usingsk_macos_get_display_name()core_driver_macos.mm: New Objective-C++ file implementing the macOS NSScreen APICMakeLists.txt: Updated to include.mmfiles in the macOS buildType of change
How Has This Been Tested?
NSScreen.localizedName:Testing Checklist
Checklist