[SWINGX-1514] Regression - WrappingProvider/Panel: background on icon even if not highlighted Created: 06/Aug/12  Updated: 21/Feb/13  Resolved: 21/Feb/13

Status: Closed
Project: swingx
Component/s: Misc Component, Renderer
Affects Version/s: 1.6.3, 1.6.4
Fix Version/s: 1.6.5-1

Type: Bug Priority: Blocker
Reporter: kleopatra Assignee: Karl Schaefer
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 0 minutes
Time Spent: Not Specified
Original Estimate: 0 minutes

Issue Links:
Dependency
depends on SWINGX-1545 JXPanel: painting breaks backwards co... Resolved
blocks SWINGX-1518 JXPanel: umbrella issue for paint pro... Closed

 Description   

to reproduce, see f.i. the demo, any where a JXTree/Table has a background highlighter: the background color of the highlighted rows is showing behind all icons, even those in not-highlighted rows

Regression, because it didn't behave like that in 1.6.2

need to track where it really happened: candidates are the WrappingIconPanel itself, or maybe the JXPanel it extends



 Comments   
Comment by kleopatra [ 07/Aug/12 ]

probably happened around #3880/#3884 which added extendsOpacity property to WrappingProvider/IconPanel

Comment by kleopatra [ 07/Aug/12 ]

Actually, the addition of the extendsOpacity seems to be only accidentally at the same time as the underlying problem (see below): the basic idea of the extendsOpacity is to allow the icon to be opaque (if the wrapped component is as well) such that background decorations extend over the icon as well as over the wrapped. What's happening now is that is the decoration always flows over the icon, even if not set on the comp By setting it to true, it's possible to at least prevent the background of the other row shine through.

Looks like the actual culprit is commit #3789 of JXPanel, something in the opacity magic running wild. Using an earlier version of JXPanel or a plain JPanel doesn't have this issue.

uppen priority (as it might effect a broader range of applications) and re-assign to Karl - don't want to step into that dungeon

Comment by Karl Schaefer [ 07/Aug/12 ]

SWINGX-1514: Use a slightly different mechanism to ensure that we paint semi-transparent backgrounds.

swingx-core/src/main/java/org/jdesktop/swingx/JXLabel.java
swingx-core/src/test/java/org/jdesktop/swingx/JXPanelTest.java
swingx-core/src/main/java/org/jdesktop/swingx/JXPanel.java
swingx-core/src/main/java/org/jdesktop/swingx/JXButton.java
swingx-core/src/main/java/org/jdesktop/swingx/SwingXUtilities.java

Committed revision 4225.

Comment by kleopatra [ 07/Aug/12 ]

cool - that was quick

Though not yet entirely fixed:

  • the part that's fixed is that the highlight background now isn't longer showing in the not-highlighted rows
  • that part that's not fixed is allowing the icon background being opaque

will have to check that this remaining part is a JXPanel issue, though, and not a side-effect of the fixed #1188 as of today. Will come back tomorrow

Cheers
Jeanette

Comment by kleopatra [ 07/Aug/12 ]

quick check:

  • temporary changed WrappingIconPanel to extend JPanel instead of JXPanel fixes the second part as well, or
  • temporary changed type of its iconLabel to JLabel still exhibits the second part

so on the whole, I think there's still some glitch in the JXPanel's opacity handling

Cheers
Jeanette

Comment by Karl Schaefer [ 07/Aug/12 ]

While this is a good check for JXPanel's opacity issues, why are we using JXPanel in a renderer? That seems really strange to me.

Do you actually have code that I can run? Everything seems fine in SwingXSet....a small, runnable demo would help.

Comment by kleopatra [ 08/Aug/12 ]

As to the why: can't really remember - probably the idea to allow painters covering the whole of the wrapper. Never tried that, though

Disagree that everything seems fine in SwingXSet, but guilty of sloppily describing the not-fine, second part as of my last comment, so trying again:

look at the Highlighter demo, JXTree tab and select a node
expected: icon background is tree color (at least for not-motif)
actual: icon background is selection color

The iconLabel is transparent by default (can be configured to have the same opacity as the main component) as is the WrappingIconPanel, nevertheless the latter seems to paint the background always.

There's a visual check in RendererIssues as well (can't believe I forgot to mention it here ... cough) interactiveTreeRendererSimple that demonstrates the issue as well: toggling the extendsOpacity property should show the highlight/selection color under the icon as well (or not)

Cheers
Jeanette

Comment by kleopatra [ 08/Aug/12 ]

okay, think I found it: culprit seems to be the BackgroundPainter:

if (object.isOpaque() || 
    object instanceof AlphaPaintable ||  UIManager.getLookAndFeel().getID().equals("Nimbus")) {


// should probably be something like

if (object.isOpaque() || 
   (object instanceof AlphaPaintable && 
   ((AlphaPaintable) object).getAlpha() < 1f) || 
              UIManager.getLookAndFeel().getID().equals("Nimbus")) {

there might be more to it, but that solves the last part on face value.

curious: why create a new painter on each call in XUtilities.paintBackground? Can't we re-use a single one, all access is supposed to happen on the EDT, so shouldn't it be safe enough?

Cheers
Jeanette

Comment by Karl Schaefer [ 08/Aug/12 ]

That code makes me sad that you of all people have fallen prey to non-opaque equals no background.

I guess that satisfies the misconception while dealing with how JXPanel wants to paint.

We build a new one because we used to do it differently and I haven't isolated the code better.

Comment by kleopatra [ 08/Aug/12 ]

no need to be sad on my behalf - I'm aware of the difference The problem with the code I cited is, that the painter paints the background unconditionally if the target is a AlphaPaintable (which is always the case for a JXPanel)

(Mis-)using opaque, is how it used to be in core - for the path of least surprise we probably should stick to it (the change did break existing code, didn't it .. Or any other marker on the paintable so the background painter can decide whether or not it should paint.

However we do it, I think we have to support it in some manner. Moving the burden to client code - which could set a do-nothing painter - doesn't feel quite right

Comment by kleopatra [ 13/Aug/12 ]

hacked WrappingIconPanel to not paint its background (by installing a dummy painter), revisit once there is some formal mechanism to prevent super painting always

committed as of revision #4232

Comment by Karl Schaefer [ 13/Aug/12 ]

Why are you checking in patch code? I thought we were trying to fix this? I am now really confused.

Comment by kleopatra [ 14/Aug/12 ]

beause it's important to have it working now

It's probably two issues

  • WrappingIconProvider allowing to configure whether the background of the wrapped component extends to the icon as well
  • JXPanel supporting configuration of whether or not the background should be painted

As long as the second isn't available, the former can be handled by a the patch. As soon as the second will be available, the patch can be thrown out.

Comment by Karl Schaefer [ 13/Nov/12 ]

SWINGX-1514: Added a follow-up check to the AlphaPaintable clause to determine if the current alpha is less than 1. We assume if the alpha is 1 and the component is not opaque that we should prevent background painting. Also removed and inlined BackgroundPainter, which is no longer useful (was part of a previous iteration of this logic).

swingx/swingx-core/src/main/java/org/jdesktop/swingx/renderer/WrappingIconPanel.java
swingx-core/src/main/java/org/jdesktop/swingx/SwingXUtilities.java
swingx-core/src/main/java/org/jdesktop/swingx/BackgroundPainter.java

Committed revision 4252.

Comment by kleopatra [ 13/Feb/13 ]

still virulent for Nimbus (probably other synth-based lafs as well)

runnable code now is in RendererVisualCheck (it used to work with the hack, so moved from issues to check - didn't check again when you removed the hack , same methods as earlier.

Comment by kleopatra [ 13/Feb/13 ]

the underlying issue probably is the same as 1545

Comment by Karl Schaefer [ 21/Feb/13 ]

Looks fine when I run the visual check.

Generated at Tue Mar 31 17:41:10 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.