udifuchs / icc-brightness

Control OLED display brightness by applying ICC color profiles.
MIT License
172 stars 25 forks source link

Choose first embedded screen instead of first screen #32

Open rb3ckers opened 4 years ago

rb3ckers commented 4 years ago

When I have my laptop (Dell XPS) connected to a second screen sometimes icc-brightness picks that screen instead of the embedded screen.

This change fixes that. See also this closed PR that had the same fix but included a bunch of other changes as well: https://github.com/udifuchs/icc-brightness/pull/30.

tartansandal commented 3 years ago

@rb3ckers I ran into a similar problem with my XPS. Unfortunately, the order of the display data was unpredictable for me. You may want to check out my fix on #34.

mradke commented 3 years ago

The patch works on my HP Spectre. I think it is sensible to choose the first embedded screen. @tartansandal why is the order important with this approach here? This would - as far as I understand it - only be problematic if your device has multiple embedded displays.

tartansandal commented 3 years ago

@mdrake

why is the order important with this approach here?

A bit of poor communication on my behalf. Apologies. I was just reiterating the coding issue that both PRs were addressing in different ways.

If you are only interested in controlling the brightness on your laptop, then yes, choosing the first embedded screen is the most sensible approach. However, if you want to keep open the potential for controlling external monitors using the same tool, then that would be problematic.

edit: Though, now that I've had my morning coffee and a think about it, picking the first embedded screen if there is no explicit target does make sense. I'll incorporate that into my MR. :smile: