ProgressIndicator indeterminate transition bugs

Wed Jul 13 21:29:34 UTC 2016

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
>>>
>>>