kirill-grouchnikov / radiance

Building modern, elegant and fast Swing applications
BSD 3-Clause "New" or "Revised" License
794 stars 89 forks source link

[Radiance] Support screens with different scaling factors #329

Closed Christoph-GEM closed 3 years ago

Christoph-GEM commented 3 years ago

Hi, we are using the ribbons facing a scale related problem:

Setup:

Screen 2 is the main screen. On screen 1, everything seems to be fine. But when we are dragging or starting the application to screen 2, some RichToolTip titles and/or descriptions are cut off like in provided the screenshot. (In this screenshot the description should be "Polygon") We tested it Radiance 3.5.1 and Radiance 4.0-SNAPSHOT. Do you have any hint?

ribbon_description_cut_off

While we were investigating this issue, we noticed another thing (only when using Radiance 4.0-SNAPSHOT): Assuming the setup described above. When we are using screen 1 as main screen, we get an IllegalArgumentException (the file contains the whole exception) when dragging the application to screen 2. In this case, the UI components e.g. ribbon components are not drawn until we move the cursor above them.

Exception in thread "AWT-EventQueue-0" java.lang.IllegalArgumentException: Keyframe fractions must be increasing: 1.0 at java.desktop/java.awt.MultipleGradientPaint.<init>(MultipleGradientPaint.java:184) at java.desktop/java.awt.LinearGradientPaint.<init>(LinearGradientPaint.java:285) at java.desktop/java.awt.LinearGradientPaint.<init>(LinearGradientPaint.java:243) at java.desktop/java.awt.LinearGradientPaint.<init>(LinearGradientPaint.java:138) at org.pushingpixels.substance.internal.painter.SeparatorPainterUtils.paintSeparator(SeparatorPainterUtils.java:252) at org.pushingpixels.flamingo.internal.substance.ribbon.ui.SubstanceRibbonUI$SubstanceTaskToggleButtonsHostPanel.paintTaskOutlines(SubstanceRibbonUI.java:116) at org.pushingpixels.flamingo.internal.ui.ribbon.BasicRibbonUI$TaskToggleButtonsHostPanel.paintComponent(BasicRibbonUI.java:838) at org.pushingpixels.flamingo.internal.substance.ribbon.ui.SubstanceRibbonUI$SubstanceTaskToggleButtonsHostPanel.paintComponent(SubstanceRibbonUI.java:126) at java.desktop/javax.swing.JComponent.paint(JComponent.java:1074)

Thanks in advance.

And BTW thanks for your great work.

Scaling_issue_IllegalArgumentException.txt

kirill-grouchnikov commented 3 years ago

The sizing / layout logic for the rich tooltip panel is in https://github.com/kirill-grouchnikov/radiance/blob/sunshine/flamingo/src/main/java/org/pushingpixels/flamingo/internal/ui/common/BasicRichTooltipPanelUI.java, lines 177-307 for computing the preferred size, and 310-491 for laying out the content.

I don't have any access to multi-monitor environment in the near future. Perhaps you can start by setting some background colors on the different parts of the rich tooltip and seeing how it looks like when it "works" and how it looks like when it doesn't. That should be a good entry point in analyzing the logic and narrowing it down. It might be around the usage of FontRenderContext and LineBreakMeasurer that maybe need to be "aware" of display configuration.

Not sure what's going on with the other crash. Again, if it's only happening in the multi-monitor environment, I don't have any timeline on when I'd be able to reproduce and analyze that on my side. SubstanceTaskToggleButtonsHostPanel.paintTaskOutlines would be the starting point around https://github.com/kirill-grouchnikov/radiance/blob/sunshine/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/ribbon/ui/SubstanceRibbonUI.java#L111 to see the bounds of the task toggle button on "regular" screen, and the bounds on the secondary screen.

Christoph-GEM commented 3 years ago

All right, thanks for your reply, we will look into it.

kirill-grouchnikov commented 3 years ago

https://github.com/kirill-grouchnikov/radiance/commit/4288406394ff0618df60d8baa6723378cdd14e4c should address the crash in SeparatorPainterUtils

Christoph-GEM commented 3 years ago

Hi, yes your fix prevents the crashing. Thank you.

We noticed that there are a few more UI issues which are related to multiple screens with different scaling. (Button borders, Tooltip borders and the curved "x" (close icon))

The first thing to mention is the NeonCortex.getScalefactor()/UIUTIL.getScalefactor() methods are used multiple times to get the scalefactor. It is caching the scalefactor, once it is determined. And it does not consider which screen is used for this moment. We changed it locally so that the method tries to get the current window to get the right scale and as fallback it takes any window and as further fallback it uses the old mechanism.

The second thing to mention is that really often (buffered) images are cached to once they have been created. There can be also be problems with that, according to my research.

We removed each caching mechanism I found. (Which could be related to scale issues) Maybe they should be cached screen wise?

We could resolve the "..." at the ToolTips and the border issues. We could not resolve the curved "x" visualizing issue, yet.

What do you think about these changes in general?

kirill-grouchnikov commented 3 years ago

Removing caching altogether is going to severely affect the performance of the library, so that's not an option.

Agree about the getScaleFactor. Supporting multiple screens hasn't been a priority to me, some of it coming from not having multiple screens in my setup, and some due to the lack of interest from the users.

Probably the right way forward would be to include the screen scale factor in every LazyResettableHashMap that stores BufferedImages. Looked at a couple of places, and those get a JComponent that can be queried for its getGraphicsConfiguration().getDevice().getDefaultConfiguration().getDefaultTransform().

What are the specific places where you removed the caching? I can start with those in the dev branch so that you can verify that such a change works in your setup.

Christoph-GEM commented 3 years ago

These are the places where the cache is checked, but there are more for sure. (sorry, I don't know how to disable the code preview for each reference)

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/common/GlowingResizableIcon.java#L90

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/common/TransitionAwareResizableIcon.java#L150 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/common/TransitionAwareResizableIcon.java#L199

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/utils/CommandButtonBackgroundDelegate.java#L169 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/flamingo/src/main/java/org/pushingpixels/flamingo/internal/substance/utils/CommandButtonBackgroundDelegate.java#L211

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/api/painter/decoration/ClassicDecorationPainter.java#L88

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/api/painter/decoration/ImageWrapperDecorationPainter.java#L261

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/ui/SubstanceCheckBoxUI.java#L138

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/ui/SubstanceTabbedPaneUI.java#L763

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/SubstanceMetricsUtilities.java#L59 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/SubstanceMetricsUtilities.java#L76

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/border/SubstanceBorder.java#L179

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/border/SubstanceTableCellBorder.java#L136 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/border/SubstanceTableCellBorder.java#L172

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/border/SubstanceTextComponentBorder.java#L135

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/ArrowButtonTransitionAwareIcon.java#L156 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/ArrowButtonTransitionAwareIcon.java#L224

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/CheckBoxMenuItemIcon.java#L125 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/CheckBoxMenuItemIcon.java#L168

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/HighlightableTransitionAwareIcon.java#L124 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/HighlightableTransitionAwareIcon.java#L176

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/RadioButtonMenuItemIcon.java#L121 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/RadioButtonMenuItemIcon.java#L164

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/SubstanceIconFactory.java#L207 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/SubstanceIconFactory.java#L243 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/SubstanceIconFactory.java#L822

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/TransitionAwareIcon.java#L172 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/utils/icon/TransitionAwareIcon.java#L224

https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/widget/animation/effects/GhostPaintingUtils.java#L115 https://github.com/kirill-grouchnikov/radiance/blob/e50b39b5b202822582b8b66c7a8b61eb3cbc453c/substance/src/main/java/org/pushingpixels/substance/internal/widget/animation/effects/GhostPaintingUtils.java#L148

If you are interested in the UIUtil-changes: https://github.com/kirill-grouchnikov/radiance/compare/sunshine...Christoph-GEM:sunshine

Thanks for your help.

kirill-grouchnikov commented 3 years ago

Yes, it's going to touch a lot of files. I'll first need to measure the performance implications on a single-screen setup with https://github.com/kirill-grouchnikov/radiance/blob/sunshine/docs/tools/lightbeam/lightbeam.md

kirill-grouchnikov commented 3 years ago

This will take a bit of time, as this is going to touch pretty much all the UI delegates and components that use offscreen images for faster rendering. I'm going to edit the title of this bug to better reflect the work.

kirill-grouchnikov commented 3 years ago

Note that while this is WIP, some visuals (especially in Flamingo components) will be broken

Christoph-GEM commented 3 years ago

Note that while this is WIP, some visuals (especially in Flamingo components) will be broken

Yes that is totally clear. Thank you really much...

kirill-grouchnikov commented 3 years ago

Can you try the latest 4.0-SNAPSHOT in your environment?

Christoph-GEM commented 3 years ago

We tried the latest 4.0-SNAPSHOT:

Black line completely drawn: black line after hovering over a tab

Black line with interruptions: black line before hovering over a tab

Draggable view icons: Icon shapes of draggable view look a bit wrong

Window title bar icons: Icon shape of the  X looks a bit wrong

TabbedPane icons: Tabbed pane X looks a bit wrong

kirill-grouchnikov commented 3 years ago

So pretty much all of these fall under https://github.com/kirill-grouchnikov/radiance/issues/39 for which I don't have a solution right now (see my comments there).

Out of the three X icons only one seems to come from Substance. The other two come from custom components (from maybe docking windows library and some kind of a custom shaped tab?). Looks like those components "suffer" from the same issues under fractional metrics that Substance does. Which is in the same area as that divider line (which is between the title pane and the toggle task buttons). Probably you see similar artifacts in some undecorated frames that Substance provides borders for.

In general, in the context of that linked issue, I think the only path to resolve it is to move completely away from offscreen images and draw things directly on Graphics provided to the painting methods in all the UI delegates. I don't do any image caching in https://github.com/kirill-grouchnikov/aurora and it's certainly technically feasible to combine those layers at runtime without using intermediate images - to some extent.

First, anything "fancy" that would involve soft clipping, blurred highlights etc would be off the table. Second, not using cached offscreen images is going to be a big performance hit for Substance, and some of the flexibility provided by the painters layer of Substance would need to be sacrificed. Such an approach would play well with flatter, single-color fills or simpler gradients that seem to be popular these days, but of course things might look different in 5-10-15 years.

Can you check in your codebase where https://user-images.githubusercontent.com/77669255/112703635-f49bf700-8e97-11eb-93ab-37d1668ac03a.png and https://user-images.githubusercontent.com/77669255/112703624-eea61600-8e97-11eb-8479-d02caf89d436.png come from? I'm pretty sure that these are not Radiance components / icons.

Christoph-GEM commented 3 years ago

First, you are right, the most issues are adressed by https://github.com/kirill-grouchnikov/radiance/issues/39, I forgot that issue.

And you are also right with the icons, they are comming from the JIDE library. For me it wasn't that obvious because they look quite different, when we don't use Substance. I attached their appearance without using Substance (they look different but also a not perfect)

Icons without using Subtstance

The tabbed pane X looks the same as in Substance, sorry for mentioning this X... It also comes from JIDE.

Anyway, thank you for taking time for the investigation and the answering

kirill-grouchnikov commented 3 years ago

The difference in rendering the X icons is probably due to JIDE using some of the UIManager entries like colors or borders to try and make the icons look "consistent" with the current look-and-feel. That can only work to a certain degree, which is the main reason why Flamingo only supports Substance, and why Substance doesn't do any public-facing client properties any more.

Closing this since the original issue of caching using the "global" scale factor has been addressed. The longer term issue with fractional scaling will be tracked by #39.