@@ Coverage Diff @@ ## master #1886 +/- ## ========================================== + Coverage 25.16% 25.41% +0.24% ========================================== Files 177 178 +1 Lines 16860 16918 +58 ========================================== + Hits 4243 4299 +56 - Misses 12617 12619 +2
|Impacted Files||Coverage Δ|
- The code in this change is very clean. It's a marked improvement in terms of readability. You used a helper to avoid repeating yourself, and it's crystal clear what you're trying to get from randr and how you're handling that result.
- It's not clear that calling randr is what we want to do, although your code provides a backup strategy, so it's not exactly harmful (if comparatively slow). We can't rely on the xrandr utility being available on host systems. It's not installed by default on non-Debian, as far as I can tell, so it's not the silver bullet you think it is.
I haven't found an explanation for when we can expect X11 extensions to be installed, but if the real feature we want is the DE's choice of primary display, we might opt to prefer the current display instead. If X11 doesn't provide a utility for that, it's normally just the display that has the mouse on it.
@JoshDreamland Good news and bad news. The good news is, I am no longer relying on the cli to do this and I'm only using C++. The bad news, this runs almost exactly just as slow as the cli using native code, so I had to add back the static caching to prevent this, at the same price to pay.
@JoshDreamland Added SDL support and placeholders for Windows support. SDL works differently than X11. X11 measures the position/size of the primary monitor. SDL only has a way to measure the monitor which has the center of the window on it, according to the docs. Also this is ready for review.
@JoshDreamland now passing and ready to be merged.