ProgressIndicator indeterminate transition bugs

Sat Jul 16 13:34:50 UTC 2016

Jonathan and Chien are correct. In general, this is not something we 
would backport since they are not regressions nor trivial fixes.

-- Kevin

Jonathan Giles wrote:
> As Chien notes, I don't think there is a strong argument for back 
> porting here. Back porting is extremely restricted now (I don't know 
> the precise amount of restriction, but my instinct says "a lot"). 
> Kevin can confirm once he is back from vacation (next week), but I 
> wouldn't hold out much hope.
>
> Just think of this as Yet Another Reason To Upgrade To JDK 9 (or 
> YARTUTJDK9 for easy remembering).
>
> -- Jonathan
>
> On 14/07/16 5:00 AM, Chien Yang wrote:
>> Hi Diego,
>>
>> As far as I'm aware these fixes aren't candidates for back port to 8. 
>> We will have to evaluate them if you can provide us a good 
>> justification for doing so.
>>
>> Thanks,
>> - Chien
>>
>> On 7/13/16, 4:40 AM, Cirujano Cuesta, Diego wrote:
>>> Hi Chien, Jonathan,
>>>
>>> I saw treeNode and the progress indicator issues are all done as 
>>> talked, GREAT!
>>>
>>> Are they planned to be included in Java 8?
>>>
>>> Cheers,
>>> Diego
>>>
>>> -----Original Message-----
>>> From: Chien Yang [mailto:chien.yang at oracle.com]
>>> Sent: Dienstag, 8. März 2016 20:41
>>> To: Cirujano Cuesta, Diego<diego.cirujano-cuesta at zeiss.com>; 
>>> jonathan.giles at oracle.com; openjfx-dev at openjdk.java.net
>>> Subject: Re: ProgressIndicator indeterminate transition bugs
>>>
>>> You may track the progress of this work with this JIRA for now. We 
>>> are still investigating the scope and impact of this work. We may 
>>> decide to create a new JIRA and resolve this as a duplicate if we 
>>> decide to go with a better naming.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8090322
>>>
>>> - Chien
>>>
>>>
>>> On 03/08/16 01:58, Cirujano Cuesta, Diego wrote:
>>>> Great! In that case forget the patch I made, it doesn’t make sense 
>>>> until the treeVisible property is implemented. Is there any jira 
>>>> ticket that I could follow?
>>>>
>>>> Thank you Jonathan.
>>>>
>>>> From: Jonathan Giles [mailto:jonathan.giles at oracle.com]
>>>> Sent: Dienstag, 8. März 2016 10:01
>>>> To: Cirujano Cuesta, Diego; openjfx-dev at openjdk.java.net
>>>> Subject: RE: ProgressIndicator indeterminate transition bugs
>>>>
>>>> The plan is that a new public API will be developed that matches 
>>>> the expectations, whilst retaining the existing treeVisible 
>>>> semantics in that API. Chien will be looking into this in the 
>>>> coming weeks. At that point we can properly fix these issues.
>>>>
>>>> -- Jonathan
>>>> Sent from a touch device. Please excuse my brevity.
>>>> On 8 March 2016 21:53:07 GMT+13:00, "Cirujano Cuesta, 
>>>> Diego"<diego.cirujano-cuesta at zeiss.com<mailto:diego.cirujano-cuesta at zeiss.com>> 
>>>> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> As I understood in the comments(JDK-8094829, JDK-8094078), 
>>>> treeVisible is buggy. Is it still buggy? If yes, is there intention 
>>>> to fix the treeVisible behavior? In case of fixing this it could be 
>>>> used to fix this issue. If I am not wrong, treeVisible is still 
>>>> used by Node, SwingNode and MediaView.
>>>>
>>>> @Jonathan As you mentioned in JDK-8094829 "having a correctly 
>>>> working treeVisible property would be immensely useful." And I am 
>>>> completely agree and I would like to add that having in the public 
>>>> API would be even better :-).
>>>>
>>>> Thank you,
>>>> Diego
>>>>
>>>> -----Original Message-----
>>>> From: Jonathan Giles [mailto:jonathan.giles at oracle.com]
>>>> Sent: Sonntag, 28. Februar 2016 22:54
>>>> To: Cirujano Cuesta, Diego;
>>>> openjfx-dev at openjdk.java.net<mailto:openjfx-dev at openjdk.java.net>
>>>> Subject: Re: ProgressIndicator indeterminate transition bugs
>>>>
>>>> If you can, are you able to file bug reports for these?
>>>>
>>>> -- Jonathan
>>>>
>>>> On 29/02/16 10:49 AM, Cirujano Cuesta, Diego wrote:
>>>>
>>>>    Hi all,
>>>>
>>>>    We found two important bugs in ProgressIndicator that are 
>>>> related with the following tickets:
>>>>
>>>>    https://bugs.openjdk.java.net/browse/JDK-8094829
>>>>    https://bugs.openjdk.java.net/browse/JDK-8094078
>>>>
>>>>    Now are quite critical because in a 4K monitor may cause 
>>>> OutOfMemoryException.
>>>>
>>>>    Using the following example:
>>>>    "
>>>>    public class JFXMain extends Application{
>>>>
>>>>         @Override
>>>>         public void start(Stage primaryStage) throws Exception {
>>>>             HBox root = new HBox();
>>>>             ToggleButton toggleButton = new ToggleButton();
>>>>             ProgressIndicator progressIndicator = new 
>>>> ProgressIndicator(ProgressIndicator.INDETERMINATE_PROGRESS);
>>>>             StackPane stackPane = new StackPane(progressIndicator);
>>>> stackPane.visibleProperty().bind(toggleButton.selectedProperty());
>>>>             root.getChildren().addAll(toggleButton, stackPane);
>>>>             primaryStage.setScene(new Scene(root));
>>>>             primaryStage.show();
>>>>         }
>>>>    }
>>>>    "
>>>>
>>>>    ** First bug **
>>>>
>>>>    Starting the Progress Indicator with indeterminate progress will
>>>>    trigger: rebuildTimeline by ProgressIndicatorSkin and in line 599
>>>>    start the animation even is not shown already:
>>>>    indeterminateTransition.playFromStart();
>>>>
>>>>    ** Second bug **
>>>>
>>>>    With the last commits in ProgressIndicator, as commented in 
>>>> JDK-8094829, the listeners do not care about the real visibility of 
>>>> the node(before it was used impl_treeVisibleProperty()). The 
>>>> consequence is that the ProgressIndicator in the example won´t be 
>>>> stopped.
>>>>
>>>>    I can imagine that impl_treeVisibleProperty() should not be used 
>>>> and Jonathan G. said: " but I can't reliably fix that without an 
>>>> API such as what I expect the treeVisible API should do." But we 
>>>> did not find such alternative property.
>>>>
>>>>    The solution we though is the usage of internal tree visible 
>>>> property like this:
>>>>
>>>>    1. Modify method:
>>>>    "
>>>>     protected void updateAnimation(boolean isTreeVisible) { "
>>>>    2. Remove current calls to method in
>>>>    "
>>>>     @Override protected void handleControlPropertyChanged(String p) {
>>>>             super.handleControlPropertyChanged(p);
>>>>
>>>>             if ("INDETERMINATE".equals(p)) {
>>>>                 initialize();
>>>>             } else if ("PROGRESS".equals(p)) {
>>>>                 updateProgress();
>>>>             }
>>>>          }
>>>>    "
>>>>    3. Add listener at the end of the IndeterminateSpinner 
>>>> contructor the visibility listener:
>>>>    "
>>>>             private IndeterminateSpinner(boolean spinEnabled, Paint 
>>>> fillOverride) {
>>>>      [...]
>>>>                 impl_treeVisibleProperty().addListener((obs, 
>>>> oldVal, newVal) ->{
>>>>                     updateAnimation(newVal);
>>>>                 });
>>>>       }
>>>>    "
>>>>
>>>>    What do you think?
>>>>
>>>>    Additional note: I would like to add one more thing. I think 
>>>> that could be very good a property ReadOnlyBooleanProperty 
>>>> treeVisibleProperty() available in all Nodes.
>>>>
>>>>    Please let me know if we can do something else.
>>>>
>>>>    Diego
>>>>
>>>>
>