Cannot set exact RGB color
ed-erwin-tf opened this issue · 5 comments
Radiance 7.0.2, Theming, Java 11, Windows 10.
Using the color chooser dialogs, it can be impossible to specify an exact RGB value.
For example, if you try to select (165, 85, 87) it will actually result in (163, 85, 87).
(The tool-tip shows the actual value that will be returned when you click OK.)
If you try to select (115, 91, 169) it will result in (114, 91, 168).
Most people won't notice, but sometimes people do care about exact RGB values.
The bug is fairly simple. In simplified terms, the ColorWheelChooser class is getting notified when the color changes in any other color chooser; it sets that value on an instance of HSVColorSliderModel; that change is then being broadcast back to the other color choosers, but not always exactly the same color because of conversion between RGB and HSV and back to RGB.
One fix is to simply turn off the ColorWheelChooser using UIManager.put("ColorChooser.defaultChoosers", ...);
Another fix is to modify the updateChooser() and setColorToModel(Color c) methods in the ColorWheelChooser to avoid recursion, or create a simple sub-class to do the same:
public class ColorWheelChooser2 extends ColorWheelChooser implements UIResource {
protected int recursion;
public ColorWheelChooser2() {
super();
recursion = 0;
}
@Override
public void updateChooser() {
recursion++;
super.updateChooser();
recursion--;
}
@Override
public void setColorToModel(Color color) {
if (recursion == 0) {
getColorSelectionModel().setSelectedColor(color);
}
}
}
An interesting find. So it may very well be that the round trip of RGB -> custom -> RGB in any implementation class of ColorSliderModel
is not guaranteed to result in the same value as it has started with, or maybe not for every possible starting int
value.
One idea might be to always use Color color
as the "source of truth" for the currently selected color in any panel that is backed by ColorSliderModel
, since eventually JColorChooser
will be returning a Color
object. Then the issue of being able - or not able - to precisely select a very particular color is pushed down to the individual color selector panels. So let's say the color wheel is backed by an HSB model, but even though the sliders represent the hue-brightness-saturation, the backing model will still be a Color
. In that case, the broken round trip is still there, but instead of RGB -> HSB -> RGB it is HSB -> RGB -> HSB confined only to that panel, and perhaps the user expectation of being able to choose precise values for HSB cannot be met in any case, again since JColorChooser
only works with RGB model as effectively implemented by the AWT Color
class.
My understanding is that the "source of truth" is the Color
in the ColorSelectionModel
in the JColorChooser
. And that is stored as rgb (with alpha).
The biggest thing to worry about is that we don't get a loop where change in panel 1 causes update in panel 2 which causes update in panel 1 which causes update in panel 2, etc. (That is what the error is, though it is only bouncing once. ) The "recursion" variable I added prevents this. (There is already a similar variable in some of the other choosers.)
The method updateChooser
is called in response to a change in some other panel to the primary ColorSelectionModel
. This may cause the panel to update its internal color model, but should not cause this panel to bounce back a new change to that primary ColorSelectionModel
.
Since a java Color
is stored as RGB, we should be able to guarantee that we can run this code
Color color2 = JColorChooser.showDialog(parentComponent, dialogName, color1);
and color2 will be the same as color1 if the user simply presses "OK" without touching anything.
Without the code fix above, or something similar, that guarantee does not hold.
We cannot guarantee that if the user selects a color using HSV, then the exact same HSV values would appear when they re-open the color chooser dialog. You would have to redesign or replace the JColorChooser
class if you wanted to guarantee saving the color in anything other than RGB or in multiple possible representations. With the existing JColorChooser
we can only guarantee that color2 == color1
.
By the way, I think this issue was caused by something during the fix of issue #390 .
Let me know if this change fixes the issue for you
Thanks. That will work.