feat(macOS): libdispalydevice integration#5338
Conversation
|
Apologies, but I merged a change a little while ago that causes conflicts with this one. |
martona
force-pushed
the
integrate-macos-libdisplaydevice
branch
from
ede0ab1 to
b733db8
Compare
|
No worries, should merge cleanly now. |
ReenigneArcher
force-pushed
the
integrate-macos-libdisplaydevice
branch
from
6c46fdc to
5700431
Compare
|
Bundle ReportBundle size has no change ✅ |
| #endif | ||
| } | ||
|
|
||
| std::unique_ptr<DisplayPowerInterface> make_display_power() { |
There was a problem hiding this comment.
Wasn't this added for Windows as well?
Also, found this which should maybe replaced with the new library code?
Sunshine/src/platform/windows/display_base.cpp
Lines 241 to 248 in 9f645a9
There was a problem hiding this comment.
Yeah that's what I meant in part when I said I could make a power management pass after the interface-factories. Technically it could have been a part of this PR, but I did not want it out of control, touching multiple OSes. A note regarding it is also in the body text of the PR: it's definitely not forgotten about, it just felt out of scope.
There was a problem hiding this comment.
Obviously, I have a reading problem today. Thanks for the clarification!
There was a problem hiding this comment.
Maybe just throw a couple TODO comments in? (Sonar will complain about the TODOs, but I'll bypass it)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5338 +/- ##
==========================================
- Coverage 27.44% 27.23% -0.22%
==========================================
Files 113 113
Lines 25547 25451 -96
Branches 11244 11218 -26
==========================================
- Hits 7012 6932 -80
+ Misses 15474 15410 -64
- Partials 3061 3109 +48
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 56 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
|
Everything going red is on libdisplaydevice. devices.emplace_back(device_id, display_name, friendly_name, edid, info); rely on C++20 parenthesized aggregate initialization through The cleanest fix is to go back to libdisplaydevice and undo the SONAR fix that caused this by changing the affected Before I do that though... does this sound like the best course of action to you too? |
That sounds fine, maybe we should add an older macOS runner to the libdisplaydevice build/test matrix as well. I think there's a few other things going on as well.
I didn't check every failure though, this was only a spot check. |
|
There seem to be three failure cases. Everything else appears to be noise stemming from these:
I'll work on the libdisplaydevice-side fix and send a PR for it. Then we can bump the libdisplaydevice pin on this PR which should clear the errors. |
|
Apologies for the delay, life got in the way. With the libdisplaydevice-changes integrated and the two Sunshine-side fixes, this should now build cleanly in CI. |
Co-authored-by: Dave Lane <42013603+ReenigneArcher@users.noreply.github.com>
ReenigneArcher
force-pushed
the
integrate-macos-libdisplaydevice
branch
from
6859972 to
7c52ebc
Compare
2 participants
Description
This is a follow-up from last week's libdisplaydevice work: macOS integration in Sunshine.
macOS now uses libdisplaydevice for display enumeration as well as applying/reverting changes.
I also removed the patch that kept the display awake; Sunshine now relies on the new libdisplaydevice functionality to do the same. Note that while libdisplaydevice gained the ability to wake displays on Windows, this PR does not take advantage of it; the scope was kept to macOS.
I had to bump the libdisplaydevice submodule.
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage