-
-
Notifications
You must be signed in to change notification settings - Fork 616
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
Detect that nvidia display exists in each refresh #1169
Conversation
✅ Deploy Preview for conkyweb canceled.
|
src/nvidia.cc
Outdated
|
||
++s; | ||
} // namespace | ||
|
||
Display *nvidia_display_setting::get_nvdisplay() { | ||
if (!nvdisplay.empty()) { | ||
Display *nvd = XOpenDisplay(nvdisplay.c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing the corresponding calls to XCloseDisplay
to release the resources from XOpenDisplay
.
Alternatively, you could wrap the pointer in a small struct or class and use RAII to call XCloseDisplay
in the destructor.
EDIT: Note that the Lua API code, does close the display, but that is separate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it was bad I forgot about releasing (I made incorrect assumptions about the XDisplay without checking specs). I updated the code with smart pointer to handle this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
XCloseDisplay(dp); | ||
} | ||
|
||
using unique_display_t = std::unique_ptr<Display, decltype(&close_nvdisplay)>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great way to handle this!
@@ -366,6 +368,14 @@ struct nvidia_c_string { | |||
const int MAXNUMGPU = 64; | |||
|
|||
namespace { | |||
|
|||
// Deleter for nv display to use with std::unique_ptr | |||
void close_nvdisplay(Display * dp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Descriptions
I use laptop with dual graphics. The nvidia is used on demand with bumbleebee. Currently if the optirun as not used, the conky will abort due to missing nvidia display (typically :8) or if started when optirun running (nvidia display present) then will crash after the optirun finish due to invalid pointer.
In my change the
XOpenDisplay
is call each time when the nvidia desktop is requested. As I have no experience with X server I don't know what impact it might have on the performance.This change should not affect current behaviour in any way.
I tested it with my laptop trying various orders of starting conky, optirun and stoping optirun to find any issues. None found.
This contribution is GPLv3 licensed.