> The new
> implementation in SubstanceProgressBarUI ignores the
> value of the "animationIndex" from the base UI
> delegate.

hmmm...: ignoring == not complying to super's contract. After all, the animationIndex related methods are meant to drive the animation. I'm a bit weary doing so just because the intended use doesn't quite fit the custom animation framework.

We all agree that going-reflection is not very nice nor cleqan - but Substance' current implementation will fool subclasses as well.

Not sure what is the super's contract here. The animationIndex is being used in a few methods of BasicProgressBarUI, and those are ignored in Substance that provides its own implementation of the indefinite animation. As such, there is no need to update that property since it is not used anymore.

The main goal of Trident adoption in Substance 6.0 is to use it for everything - with zero instances of external timers / threads. This will cut down on the number of threads, as well as provide a centralized way to configure the animation parameters such as duration and ease methods.

concededly, the contract is not very strong. And using Trident consistently in the bowels of Substance sure is a good move. Nevertheless, I would prefer it to use super's (probably overridden) api (that is start/stop/set/incrementIndex) instead of just ignoring. super documents those are used to drive the animation/state, if you do it differently it's ... well ... not a peaceful evolution but violent replacement :-)

Fine, i give up. The CellRenderer philosophy is broken and shall not be fixed except by the holy mandate of the swing team.

Fine.

Btw, you should see if the gc thread is not suffering now that you've replaced the animation thread. There is a slight jerk every full cycle now i think (on the normal JProgressBar in the example kleopatra gave above).

Thanks for reporting this. Nothing to do with GC - just the visual representation of the animation is not fully aligned to be seamlessly looping. Seems to be off by a pixel or two and will be fixed shortly.

That is an incidental side effect. The new implementation in SubstanceProgressBarUI ignores the value of the "animationIndex" from the base UI delegate. All you need to do in your timer action listener is to repaint the list - or rather the specific cell. No need to call the incrementAnimationIndex() via reflection as that has zero effect on the Substance delegate.

However, once again i want to stress that this is incidental and cannot be relied on.

> That is an incidental side effect. The new
> implementation in SubstanceProgressBarUI ignores the
> value of the "animationIndex" from the base UI
> delegate. All you need to do in your timer action
> listener is to repaint the list - or rather the
> specific cell. No need to call the
> incrementAnimationIndex() via reflection as that has
> zero effect on the Substance delegate.
>
> However, once again i want to stress that this is
> incidental and cannot be relied on.
>
> Thanks
> Kirill

It's all cool, in that case i will override the model so it always returns 0 or something.

Btw, i couldn't get the same stack-trace in the my test. but i believe that trident is not the cause anyway and i think the situation is the same. I'm posting this here also, because i believe that this also happens if you exchange "SubstanceRavenLookAndFeel" for "WindowsClassicLookAndFeel" or any other for that matter. Appears to be a jdk swing bug or something. Since i know no one looks at the bug parade and i can't post bugs there for some reason, i'm posting it here, both versions.
Substance:
[code]
import java.awt.EventQueue;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.SwingUtilities;
import javax.swing.UIManager;
import javax.swing.event.ListSelectionEvent;
import javax.swing.event.ListSelectionListener;
import javax.swing.plaf.metal.MetalLookAndFeel;
import org.junit.Test;
import org.pushingpixels.substance.api.skin.SubstanceRavenLookAndFeel;

Just give all the registered listeners a chance to process the selection event before changing the look-and-feel. Wrap the logic in valueChanged() inside the SwingUtilities.invokeLater. It's always a good practice, since you cannot know which additional listeners are registered by Swing core / look-and-feel in addition to your own app listeners.

Regardless, i just put wrapped the code that contains the laf change and updateComponentTreeUI call in a runnable that i resent into the eventqueue with invokelater and it is still throwing the original exception.
Very strange.

Does the component i'm trying to change the look and feel be a JDialog (i heard they replace the eventqueue) have any relevance? I tried that with the testcase but couldn't get it to crash with after the eventqueue call was added to addListSelectionListener

Actually. No. Doing what you told me made the changes between the sun provided look and feels work fine. The repeated
[code]
java.lang.NullPointerException
at org.pushingpixels.trident.swing.SwingRepaintCallback.onTimelineStateChanged(SwingRepaintCallback.java:66)
at org.pushingpixels.trident.Timeline$Chain.onTimelineStateChanged(Timeline.java:206)
at org.pushingpixels.trident.TimelineEngine$1.run(TimelineEngine.java:610)
at org.pushingpixels.trident.TimelineEngine$TimelineCallbackThread.run(TimelineEngine.java:255)
[/code]
Exception - at the first bug report - continues, only if i try to change to a substance laf in my larger application, after using that repost to the edt trick.

Edit: if that's what you fixed, i apologize. I'm still using the same version (6.0 build from a few days ago).

The Substance issue is at [1]. I do not agree that this is the right approach at all. Even ignoring the fact that you're using the same progress bar for all the cells, you're relying on the internal implementation details of the UI delegate of the specific look-and-feel.

This is not something that you can do, and the latest 6.0dev drop of Substance has been changed to use Trident to drive the animations of the indeterminate progress bars. While you can still use reflection to get access to the property being animated (animationPosition) and the timeline which animates it (indeterminateLoopTimeline), you'll be doing it at your own risk. This change was going into 6.0dev in any case, but your issue request just sped up the inevitable.

In general, using animated components does not sit well with the renderer-based approach of Swing lists, tables and trees.

(By using a weakreference for holding the UI reference this in the animation thread. Then when the JProgressBar (or component) was gc, the UI also would, and then the get would return null -> sign for the thread to stop.)

> The cause is btw, appears to be, that the
> BasicProgressBarUI inner class Handler is, among
> other things, a HierarchyListener, receives events of
> hierarchy changes from the JList renderers (that use
> the cellrenderer).

no, not really: the problem is that you try to use some implementation detail of the ui to control something unrelated outside of its scope of responsibility ;-) Fact is that renderering components are not part of the container hierarchy ever - except for the very short instance when it is painted. Assuming something else is simply wrong. Period. No exception.

Aggravated by the fact, that what to do in case of "indeterminate" is really deeply hidden, it's considered as a wholistic property which the ui is free to fill with substate, such as f.i. animation steps. Now expecting something from that internal animator which does animate some unknown internal substate is ... wrong. Don't. Even if that internal substate would be public, using the animator would be wrong.

Exposing the semantics of substate is where an RFE might come into play (note: not the animation itself, never!) As is, the indeterminate property is a bit of a master-switch to release the ui from respecting the model: it's free to do whatever. So my take on it would be to enhance the model with that indeterminate-plus-substate and force the ui to always respect it. Dreaming of an ideal world built in black-and-white

In the real world, we are indeed stuck with going dirty - and access the internal unknow state (not the animation itself!). Not overly safe, of course, as uis are free to do whatever ... ;-) Below is a quick shot, similar to yours but moving all control over the dirtiness out off the renderer (never ever make a renderer "clever" in any way). Looks fine for metal and win, but not substance.

I'm seeing the same thing in substance (using the startAnimationTimer method instead). I was worried that two timers + the fact that using a JProgressBar as a cellrenderer starts the timer and then stops it might cause a double increment of the animation counter.

It's because the ProgressBar isn't really added to the component (or validated or something) and so the width and height return wrong values (negative ones even) on incrementAnimationCounter().
Caching the old value of the paintIndeterminate width and height fixes it.

(not a propertychangelistener. At paint indeterminate the values from JProgressBar.getWidth()/getHeight() are right, immediatly after being added to a component they aren't right, go figure).

Also this is my crappy "solution" that depends on the quantity of repaint events (increase the size of the window and the animation will be faster), and probably butchers the animation across platforms.

[code]
//class bellow shows a better solution.
//If you find out to make it to work with multiple look
//and feels reply here

ehhh ... you know that the renderer is just a passive dumb stamp of whatever the value is ;-) Wasn't aware of that basicListui property - but looks like it' plain down wrong (will check what it does tomorrow, maybe)

Let the model change and fire accordingly. Or if the "progress" is independent of the model, use SwingX and an animated highlighter (biased advise, of course, and untested)

It's not that progress is independent of the model or anything like that.
It's that the BasicProgressBarUI stops the animating thread immediately after starting it if the component is not in a hierarchy (granted, it's desirable before adding a component that it doesn't create and destroys a thread, but this should be a component property at least. This other "equivalent" class show that it is so (if you comment the stopAnimationTimer you will see it work, otherwise, you will see "STARTED" and "STOPPED" immediately after setting indeterminate to true.

To me this smells like a non-intentional limitation of the JProgressBar api. If there was some way not to require the JProgressBar to be in a hiearchy i could stamp whatever i wanted with two JProgressBar (one for discrete values another for the indeterminate values). Granted it would be harder to stop the thread at the appropriate time, but by no means impossible.
[code]
import java.awt.Component;
import java.awt.EventQueue;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.lang.reflect.Method;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.JPanel;
import javax.swing.JProgressBar;
import javax.swing.ListCellRenderer;
import javax.swing.Timer;
import javax.swing.UIManager;
import javax.swing.plaf.basic.BasicProgressBarUI;
import org.pushingpixels.substance.api.skin.SubstanceRavenLookAndFeel;

The cause is btw, appears to be, that the BasicProgressBarUI inner class Handler is, among other things, a HierarchyListener, receives events of hierarchy changes from the JList renderers (that use the cellrenderer).
After all the installs and uninstalls into the CellRenderer, the last state is always undisplayable and indeterminate so the animation thread is stopped -> no more updates for the animation.
[code]
// we don't want the animation to keep running if we're not displayable
public void hierarchyChanged(HierarchyEvent he) {
if ((he.getChangeFlags() & HierarchyEvent.DISPLAYABILITY_CHANGED) != 0) {
if (progressBar.isIndeterminate()) {
if (progressBar.isDisplayable()) {
startAnimationTimer();
} else {
stopAnimationTimer();
}
}
}
}
[/code]

Ok, someone that has a account that can do bug reports please post this as a RFE:
This class, if run as is, will spam STARTED STOPPED forever and not paint anything.
If the code that removes the extra HiearchyListener is uncommented it works as expected. It still requires the repaint for the animation to continue for some reason (the timer repaint events are not got by the JList since the JProgressBar is removed, but updates the animation values).
[code]
import java.awt.Component;
import java.awt.EventQueue;
import javax.swing.JFrame;
import javax.swing.JList;
import javax.swing.JPanel;
import javax.swing.JProgressBar;
import javax.swing.ListCellRenderer;
import javax.swing.plaf.basic.BasicProgressBarUI;

Mmmmh. Now i'm trying to find a painting stategy that doesn't consume the whole edt and processor.

The timer is running and sending repaint events to the JProgressBar. However what needs to be repainted is the JList.

I can repaint the list inside the repaint or paint component of the JProgressBar, and it's a good approach. For better performance i can also take a page out of the DefaultListCellRenderer and overload other repainting & validating methods to do nothing.

Right?

What is better in this case overloading the repaint() (that will do nothing except call jlist.repaint() ) or paintComponent(Graphics g) (that will have to call super too).

Well ok. My requirement is slightly different in that some of the JProgressBar stamps can be in indeterminate state and other not, but this shows the problem anyway and i doubt that it is different if i set it to true in the main getCellRenderer method.

Edit: also setting the value (a different value on each call) has no effect on the animation too.

[code]
/*
* To change this template, choose Tools | Templates
* and open the template in the editor.
*/