Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Theming] ColorWheel color chooser is not honoring high DPI scale factor #457

Closed
ed-erwin-tf opened this issue Jan 19, 2024 · 27 comments
Closed
Labels
7.0 - Karat Release 7.0 - 2024.H1

Comments

@ed-erwin-tf
Copy link

Version of Radiance 6.5.0

Sub-project Theming

Version of Java 11

Version of OS Windows 10

The issue you're experiencing

When using the ColorWheelChooser, the image of the color wheel is being properly scaled for high DPI monitors, but the calculation of mouse position corresponding to each color is not being scaled. This means that you can choose a location outside of the wheel. In fact you must do so if you want to choose the most saturated version of the color.

color wheel 1

@ed-erwin-tf
Copy link
Author

The fix can be made inside the ColorWheel class. Changes are needed in two locations.

In MouseHandler.update(MouseEvent e) you can do something like this:

       private void update(MouseEvent e) {
            double scaleFactor = RadianceCommonCortex.getScaleFactor(ScaledColorWheel.this);

            int x = e.getX() - getWidth() / 2;
            int y = e.getY() - getHeight() / 2;

            float r = (float) (scaleFactor * Math.sqrt(x * x + y * y));
            float theta = (float) Math.atan2(y, -x);

            int RR = (int) (Math.min(1f, r / colorWheelProducer.getRadius()) * 100f);

            model.setValue(0, 180 + (int) (theta / Math.PI * 180d));
            model.setValue(1, RR);

            // FIXME - We should only repaint the damaged area
            repaint();
        }

in paintComponent(), a similar fix is needed. (Cut paste is not working for me right now for some reason unrelated to Radiance!)

@ed-erwin-tf
Copy link
Author

Ok. In paintComponent(), do something like this:

       double r = colorWheelProducer.getRadius()/scaleFactor * model.getValue(1) / 100d;
        double angle = model.getValue(0) * Math.PI / 180d;
        int x = w / 2 + (int) (r * Math.cos(angle));
        int y = h / 2 - (int) (r * Math.sin(angle));

@ed-erwin-tf
Copy link
Author

PS: I personally think it looks better to enlarge that cursor a little bit. Something like this:

        g.setColor(Color.white);
        g.fillRect(x - 2, y - 2, 4, 4);
        g.setColor(Color.black);
        g.drawRect(x - 4, y - 4, 6, 6);

@kirill-grouchnikov
Copy link
Owner

What are the changes in paintComponent() for?

@kirill-grouchnikov
Copy link
Owner

Ah, it's for the indicator box

@kirill-grouchnikov kirill-grouchnikov added the 7.0 - Karat Release 7.0 - 2024.H1 label Jan 20, 2024
@kirill-grouchnikov kirill-grouchnikov changed the title ColorWheel color chooser is not honoring high DPI scale factor [Theming] ColorWheel color chooser is not honoring high DPI scale factor Jan 20, 2024
@ed-erwin-tf
Copy link
Author

Thanks for the quick response!

Your code fixes the problem that I identified.

Yet in your fixed code, the cursor size appears very different depending on the DPI of the display. Here I have a low-dpi version (scale factor 1.0) and high-dpi (scale factor 2.5).

hiDPI
lowDPI

@ed-erwin-tf
Copy link
Author

Ok, the cursor looks exactly the same size in those two uploaded images, but when the two images are displayed on the two monitors they are shown at the same physical size, which means the cursor in the top picture looks small and the one in the lower picture looks big.

@kirill-grouchnikov
Copy link
Owner

Oh, the size should be scaled in the 1x rendering. I'll make a quick fix in 7.0.1

@ed-erwin-tf
Copy link
Author

Thanks. Maybe wait a few days in case other bugs come in? I'll try to test 7.0.0 (if there are no breaking changes that actually affect me.)

@kirill-grouchnikov
Copy link
Owner

I'll cut 7.0.1 later in the day

@ed-erwin-tf
Copy link
Author

There is also an issue with the crayon color chooser. Here it is with scale factor 2.5.
crayons

I had not noticed this because I turn off that color panel (since I don't want to translate color names to 15 languages.)

@ed-erwin-tf
Copy link
Author

PS: In my quick tests, I didn't notice any problems upgrading from 6.5 to 7.0.

@kirill-grouchnikov
Copy link
Owner

color-chooser-2x

Looks OK for me

@ed-erwin-tf
Copy link
Author

I cannot explain why it works for you and not me! I'm Java 11 on Windows 10, two displays, one 2.5 scale (#1 in image) and one 1.0 scale (#2 in image), using "extend these displays".

I mis-spoke earlier. It is the low-dpi display that shows the problem, with scale factor 1.0, while my main screen is scale factor 2.5.
displays

@kirill-grouchnikov
Copy link
Owner

That would be in https://github.com/kirill-grouchnikov/radiance/blob/sunshine/theming/src/main/java/org/pushingpixels/radiance/theming/internal/contrib/randelshofer/quaqua/colorchooser/Crayons.java#L171

I don't have a 1x monitor, only 2x and 2.5x.

I think that it's due to the mismatch between the creation of the crayons image in RadianceImageCreator.getCrayonsImage that is using the max scale factor of all the screens, and then drawing that image on a lower-res monitor in Crayons.paintComponent respecting the scale factor of that particular screen.

@kirill-grouchnikov
Copy link
Owner

I'm thinking that the logic in Crayons.paintComponent should "scale" the scale factor to account for that, something like scaleFactor = RadianceCommonCortex.getScaleFactor(this) / RadianceCommonCortex.getScaleFactor(null). Or maybe multiply instead of divide. Or flip the order. I don't have a 1x monitor to verify which one would be the right one to do.

@ed-erwin-tf
Copy link
Author

I can try testing code changes.
(My 1.0 monitor is very old!)

@ed-erwin-tf
Copy link
Author

"Or maybe multiply instead of divide."

Neither of those works correctly. I'll try some other things....

@ed-erwin-tf
Copy link
Author

This is what works for me:

// use scaleFactorA for the image....
double scaleFactorA = RadianceCommonCortex.getScaleFactor(null);
RadianceCommonCortex.drawImageWithScale(g2d, scaleFactorA, crayonsImage, 0, 0);

// then use scale factor as before for the rest
double scaleFactor = RadianceCommonCortex.getScaleFactor(this);

@ed-erwin-tf
Copy link
Author

PS: It would be really nice if there were a setting somewhere to disable drawing the color name in this panel. That way I wouldn't need to worry about translation. Just have some flag to disable the call to "g2d.drawString(selectedCrayon.name....)"

@kirill-grouchnikov
Copy link
Owner

An API to configure the color chooser would be a nice addition, something along the lines of an enum that lists all available panels, and allows the app to choose which ones to display and in which order. I'll need to think of a way to configure the specific per-panel display options so that it's not just for the color name of the selected crayon, but something a bit more generic.

That would happen in the next release.

@ed-erwin-tf
Copy link
Author

Thanks for fixing the crayons and color wheel!

"lists all available panels, and allows the app to choose which ones to display and in which order"

Something like that already exists, though without the enum. Please don't take this ability away!

UIManager.put("ColorChooser.defaultChoosers", new String[] {ColorWheelChooser.class.getName(), SwatchesChooser.class.getName()});

I only keep the ones that scale well and don't have much (or any) text to translate.

I was also able to easily test changes, like those scaling options above, by creating a custom chooser and including it in the list:

UIManager.put("ColorChooser.defaultChoosers", new String[] {MySpecialChooser.class.getName()});

@stanio
Copy link

stanio commented Jan 27, 2024

I don't have a 1x monitor, only 2x and 2.5x.

FWIW, for testing purposes one may use -Dsun.java2d.uiScale=1.0 system property, possibly with another desired value.

@kirill-grouchnikov
Copy link
Owner

I think that would apply to all connected monitors. The environment that was needed to recreate this bug was to have one high-res monitor, and another at 1x.

@stanio
Copy link

stanio commented Jan 28, 2024

Ah, all right. I've missed the secondary issue of the color cursor not adjusting its size on a secondary monitor with a different DPI.

@ed-erwin-tf
Copy link
Author

Using -Dsun.java2d.uiScale=1.0 would cause all parts of the program to run without scaling to match the monitor resolution. Thus on a 2.5-scale monitor, text in normal font sizes would be too small to read.

But thanks for the comment. And your xbrz library could prove useful. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.0 - Karat Release 7.0 - 2024.H1
Projects
None yet
Development

No branches or pull requests

3 participants