Skip to content

Conversation

CubeTheThird
Copy link

Added a brightness variable to the gamma calculation, based on the formula used in the original redshift. As I did not also add a gamma variable, I assume this to be 1, which simplifies the equation to a few multiplications.

Added day and night brightness settings to the extension preferences.

Included patches for gnome 3.22 as currently included, as well as the most recent commit to master (as of a few days ago).

@benzea
Copy link
Owner

benzea commented Jan 23, 2017

On a general note, it would be good if you squash typo corrections into the original commits (you can use "git rebase -i" and then reorder them and use "fixup" to do so very easily). Also, I am not sure I like the fact you changed the author of the patch file, but more importantly, you should add a Signed-Off-By header (preferably with a real name) :)

The main objection to merging this from my side is missing rational why the normal brightness control for laptop monitors couldn't be extended to do the simulation on monitors that cannot be adjusted. Duplicating the feature in a way that completely bypasses the existing brightness controls seems wrong.

@CubeTheThird
Copy link
Author

Thank you for the advice about the rebase. I had used git's format-patch to generate the patches, which is why the name was changed. I have no issues leaving that, along with the sign off, as they were originally.

Normal laptop brightness control is a simple hardware switch which you can read from and write to (typically in /sys/class/backlight). Adding a 'simulation' layer on top of this implemented in software would be dependant on the desktop environment and compositor, and there is no global generic solution (that I've ever heard of) that accomplishes this. Furthermore, the hardware has limitations, since there are (normally) fixed increments at which the brightness can be adjusted, which is vendor dependent.

The purpose of my pull request is to allow fine grained control of the brightness in conjunction with the already configured colour temperature, allowing it to not only simulate hardware brightness settings, but also have it change gradually with time. This functionality is not new, as (previously stated) it's already implemented in the original Redshift project.

As for your argument of duplicating features, I could easily counter-argue that my desktop monitor has built-in settings for adjusting the colour temperature, which would equally suggest this project already duplicates functionality. The existence of this project (as well as redshift) goes to show that there is a demand for fine grained control of existing monitor settings (whether it be brightness or temperature), implemented in software, to add functionality not currently possible with the hardware alone.

@CubeTheThird
Copy link
Author

As recommended I have restored the authoring info to the patches, and applied rebase to clean up the commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants