GNOME Bugzilla – Bug 645665
Magnifier: Add brightness and contrast preferences
Last modified: 2012-05-16 22:56:18 UTC
Per the magnifier's features-in-development list (http://live.gnome.org/GnomeShell/Magnification#Features_in_Development), other display enhancements for low vision users are inverse video, contrast, and brightness controls. There is a patch for the gnome-shell magnifier for this functionality -- see 639851. When that patch was submitted, the settings schema (org.gnome.desktop.ally.magnifier) was part of gnome-shell. The schema has since moved to this project. Since these new preferences are on equal footing with the other magnifier preferences, they should be added here. There needs to be: - a preference for turning inverse video on/off, - a preference to allow users to increase/decrease the brightness of the magnified view, and - a preference to allow users to change the contrast of the magnified view.
Created attachment 184227 [details] [review] Add preference keys for magnifier inverse-video, and brightness and contrast levels The patch adds preferences to the schema for turning inverse video on/off, changing the brightness level, and the contrast level of the magnified view.
This sounds like you're just showing the implementation details through the options. Please use 3 separate keys for each of the settings, clamped between -1.0 and +1.0. That should at least make it comprehensible.
Created attachment 191030 [details] [review] Split the brightness and contrast change settings into one each for red, green, and blue. Bastien, I *think* you're suggesting to breakout, for example, the single contrast setting into three, one for red, one for green, and another for blue. Here's a version of the schema that does that. Let me know if that's you meant.
Review of attachment 191030 [details] [review]: Typos. ::: schemas/org.gnome.desktop.a11y.magnifier.gschema.xml.in.in @@ +154,3 @@ + <_description> + Represents a change to the default contrast of the red component. Zero + <_summary>Change brightness of green</_summary> lest? @@ +164,3 @@ + <_description> + Represents a change to the default contrast of the green component. + </_description> lest. @@ +174,3 @@ + <_description> + Represents a change to the default contrast of the blue component. Zero + <_summary>Change brightness of blue</_summary> lest.
Created attachment 191049 [details] [review] Fixed spelling mistakes Thanks for catching that. It's not surprising that the spell checker declared "lest" okay since it is a word. Just not the right one.
Created attachment 209353 [details] [review] Replace 'invert-brightness' with 'invert-lightness' One small change -- technically the inversion setting is a lightness inversion, not a brightness inversion. This changes the one key to 'invert-lightness', and modifies its desription as appropriate. But, I don't want this to hold things up. If it's too late for the commit, go ahead with the previous patch.
(In reply to comment #6) > ... changes the one key to 'invert-lightness', > and modifies its desription as appropriate. The functionality that uses these preferences has been marked accepted-commit-now (https://bugzilla.gnome.org/show_bug.cgi?id=639851). However, this bug is blocking. Is it possible to get a review of the above change I requested so as to move forward?
Review of attachment 209353 [details] [review]: Looks fine to me. Considering the previous version was already acn, this one should be too
The following fix has been pushed: d964420 Magnifier: Add brightness and contrast preferences
Created attachment 214218 [details] [review] Magnifier: Add brightness and contrast preferences Added keys invert-lightness, brightness-red, brightness-green, brightness-blue, contrast-red, contrast-blue, and contrast-green.