RE: [EXTERNAL] Re: RE: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-26 Thread David Grieve
That’s possible. I think that’s in the ballpark, at least.

If we could somehow set the cell state before it gets added back into the 
scenegraph, then the cell would be rendered correctly from the start, rather 
than having its state updated halfway through the layout. I haven’t had time to 
circle back on this, but I was looking at and around 
javafx.scene.control.Cell#updateItem and 
javafx.scene.control.Cell#updateSelected

From: Eric Bresie 
Sent: Thursday, March 26, 2020 9:01 AM
To: David Grieve 
Cc: Danny Gonzalez ; 
openjfx-dev@openjdk.java.net
Subject: [EXTERNAL] Re: RE: JDK-8177945 : Single cell selection flickers when 
adding data to TableView

Not sure if this is the cause of some of this but...

I see some similar flags like needsRecreateCells and needsRebuildCells.

In one context (around line 1025) I see a recreateOrRebuild flag based on the 
two.

Is it possible something similar is needed around here to only do something 
once if either is set?

Maybe it’s doing the reset twice as part of recreate and rebuild.

Eric Bresie
ebre...@gmail.com<mailto:ebre...@gmail.com>


On March 10, 2020 at 11:48:27 AM CDT, David Grieve 
mailto:david.gri...@microsoft.com>> wrote:
The flickering is happening because of the way VirtualFlow reuses cells. The 
selected state gets cleared and reset, which is the flicker.
This change fixed the flickering for me. I'm not sure why the index of a cell 
was being set to -1, but the effect of doing so is that when the
cell is pulled out of the pile, it looks like a new cell that has to have its 
index set, selected state set, etc, etc. By commenting out the
updateIndex(-1), VirtualFlow can reuse the cell - setting the index, selected 
state, etc turns into noops (more or less).

Again, there may have been a good reason for setting index to -1 in this 
condition. More investigation needs to be done.

diff --git 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
index 728e7c5513..a58859bd05 100644
--- 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
+++ 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
@@ -991,9 +991,9 @@ public class VirtualFlow extends 
Region {
lastWidth = -1;
lastHeight = -1;
releaseCell(accumCell);
- for (int i = 0, max = cells.size(); i < max; i++) {
- cells.get(i).updateIndex(-1);
- }
+// for (int i = 0, max = cells.size(); i < max; i++) {
+// cells.get(i).updateIndex(-1);
+// }
addAllToPile();
releaseAllPrivateCells();
} else if (needsReconfigureCells) {


-Original Message-
From: openjfx-dev 
mailto:openjfx-dev-boun...@openjdk.java.net>>
 On Behalf Of
David Grieve
Sent: Friday, March 6, 2020 9:29 AM
To: Danny Gonzalez 
mailto:danny.gonza...@screamingfrog.co.uk>>
Cc: openjfx-dev@openjdk.java.net<mailto:openjfx-dev@openjdk.java.net>
Subject: RE: JDK-8177945 : Single cell selection flickers when adding data to
TableView

This might fix the issue, but I’d like to understand better what the root of the
problem is. My concern is that this fix might cause a performance regression.
I’m using the code in JDK-8177945. I want to look at what TableView does
when it adds a cell. Is there something about the selected state that
changes? Or is this just the CSS implementation recalculating styles and un-
necessarily clearing old values? Some debugging is in order.

From: Danny Gonzalez 
mailto:danny.gonza...@screamingfrog.co.uk>>
Sent: Wednesday, March 4, 2020 4:34 AM
To: David Grieve mailto:david.gri...@microsoft.com>>
Cc: openjfx-dev@openjdk.java.net<mailto:openjfx-dev@openjdk.java.net>
Subject: [EXTERNAL] Re: JDK-8177945 : Single cell selection flickers when
adding data to TableView

Hi David,

Just wanted to add some more information.

The cell selection flashing issue goes away If I put back the following code in
the layout function in Parent.java:

//
// One idiom employed by developers is to, during the layout pass,
// add or remove nodes from the scene. For example, a ScrollPane
// might add scroll bars to itself if it determines during layout
// that it needs them, or a ListView might add cells to itself if
// it determines that it needs to. In such situations we must
// apply the CSS immediately and not add it to the scene's queue
// for deferred action.
//
// Note that we only call processCSS if the flag is update. If the
// flag is DIRTY_BRANCH, then the whatever children need UPDATE
// will be visited during this layout anyway. On the next pulse,
// doCSSPass will clean up the DIRTY_BRANCH nodes.
//
if (cssFlag == CssFlags.UPDATE) {
processCSS();
}

This code was originally added in in the following commit:

commit e76b5755d4d2752037cc95eb75cb2615a740cc30
Author: David Grieve
mailto:dgri...@openjdk.org<mailto:dgri...@openjdk.org%3cmailto:dgri...@openjdk.org>>>
Date: Thu Apr 10 15:40:34 20

Re: RE: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-26 Thread Eric Bresie
Not sure if this is the cause of some of this but...

I see some similar flags like needsRecreateCells and needsRebuildCells.

In one context (around line 1025) I see a recreateOrRebuild flag based on the 
two.

Is it possible something similar is needed around here to only do something 
once if either is set?

Maybe it’s doing the reset twice as part of recreate and rebuild.

Eric Bresie
ebre...@gmail.com
> On March 10, 2020 at 11:48:27 AM CDT, David Grieve 
>  wrote:
> The flickering is happening because of the way VirtualFlow reuses cells. The 
> selected state gets cleared and reset, which is the flicker.
> This change fixed the flickering for me. I'm not sure why the index of a cell 
> was being set to -1, but the effect of doing so is that when the
> cell is pulled out of the pile, it looks like a new cell that has to have its 
> index set, selected state set, etc, etc. By commenting out the
> updateIndex(-1), VirtualFlow can reuse the cell - setting the index, selected 
> state, etc turns into noops (more or less).
>
> Again, there may have been a good reason for setting index to -1 in this 
> condition. More investigation needs to be done.
>
> diff --git 
> a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
>  
> b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> index 728e7c5513..a58859bd05 100644
> --- 
> a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> +++ 
> b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> @@ -991,9 +991,9 @@ public class VirtualFlow extends 
> Region {
> lastWidth = -1;
> lastHeight = -1;
> releaseCell(accumCell);
> - for (int i = 0, max = cells.size(); i < max; i++) {
> - cells.get(i).updateIndex(-1);
> - }
> +// for (int i = 0, max = cells.size(); i < max; i++) {
> +// cells.get(i).updateIndex(-1);
> +// }
> addAllToPile();
> releaseAllPrivateCells();
> } else if (needsReconfigureCells) {
>
> > -Original Message-
> > From: openjfx-dev  On Behalf Of
> > David Grieve
> > Sent: Friday, March 6, 2020 9:29 AM
> > To: Danny Gonzalez 
> > Cc: openjfx-dev@openjdk.java.net
> > Subject: RE: JDK-8177945 : Single cell selection flickers when adding data 
> > to
> > TableView
> >
> > This might fix the issue, but I’d like to understand better what the root 
> > of the
> > problem is. My concern is that this fix might cause a performance 
> > regression.
> > I’m using the code in JDK-8177945. I want to look at what TableView does
> > when it adds a cell. Is there something about the selected state that
> > changes? Or is this just the CSS implementation recalculating styles and un-
> > necessarily clearing old values? Some debugging is in order.
> >
> > From: Danny Gonzalez 
> > Sent: Wednesday, March 4, 2020 4:34 AM
> > To: David Grieve 
> > Cc: openjfx-dev@openjdk.java.net
> > Subject: [EXTERNAL] Re: JDK-8177945 : Single cell selection flickers when
> > adding data to TableView
> >
> > Hi David,
> >
> > Just wanted to add some more information.
> >
> > The cell selection flashing issue goes away If I put back the following 
> > code in
> > the layout function in Parent.java:
> >
> > //
> > // One idiom employed by developers is to, during the layout pass,
> > // add or remove nodes from the scene. For example, a ScrollPane
> > // might add scroll bars to itself if it determines during layout
> > // that it needs them, or a ListView might add cells to itself if
> > // it determines that it needs to. In such situations we must
> > // apply the CSS immediately and not add it to the scene's queue
> > // for deferred action.
> > //
> > // Note that we only call processCSS if the flag is update. If the
> > // flag is DIRTY_BRANCH, then the whatever children need UPDATE
> > // will be visited during this layout anyway. On the next pulse,
> > // doCSSPass will clean up the DIRTY_BRANCH nodes.
> > //
> > if (cssFlag == CssFlags.UPDATE) {
> > processCSS();
> > }
> >
> > This code was originally added in in the following commit:
> >
> > commit e76b5755d4d2752037cc95eb75cb2615a740cc30
> > Author: David Grieve
> > mailto:dgri...@openjdk.org>>
> > Date: Thu Apr 10 15:40:34 2014 -0400 (tel:34%202014%20-0400)
> >
> > RT-36559: Some css optimizations: 1 - on impl_reapplyCSS, do not reapply
> > css to child nodes if nothing has changed. 2 - on applyCss, only look for
> > ancestor node with css state = UPDATE. 3 - only recalculate checksum of css
> > file if the

Re: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-12 Thread David Grieve
This fix causes several unit tests to fail.

From: David Grieve 
Sent: Tuesday, March 10, 2020 12:48 PM
To: David Grieve ; Danny Gonzalez 

Cc: openjfx-dev@openjdk.java.net 
Subject: RE: JDK-8177945 : Single cell selection flickers when adding data to 
TableView

The flickering is happening because of the way VirtualFlow reuses cells. The 
selected state gets cleared and reset, which is the flicker.
This change fixed the flickering for me. I'm not sure why the index of a cell 
was being set to -1, but the effect of doing so is that when the
cell is pulled out of the pile, it looks like a new cell that has to have its 
index set, selected state set, etc, etc. By commenting out the
updateIndex(-1), VirtualFlow can reuse the cell - setting the index, selected 
state, etc turns into noops (more or less).

Again, there may have been a good reason for setting index to -1 in this 
condition. More investigation needs to be done.

diff --git 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
index 728e7c5513..a58859bd05 100644
--- 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
+++ 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
@@ -991,9 +991,9 @@ public class VirtualFlow extends 
Region {
 lastWidth = -1;
 lastHeight = -1;
 releaseCell(accumCell);
-for (int i = 0, max = cells.size(); i < max; i++) {
-cells.get(i).updateIndex(-1);
-}
+//for (int i = 0, max = cells.size(); i < max; i++) {
+//cells.get(i).updateIndex(-1);
+//}
 addAllToPile();
 releaseAllPrivateCells();
 } else if (needsReconfigureCells) {

> -Original Message-
> From: openjfx-dev  On Behalf Of
> David Grieve
> Sent: Friday, March 6, 2020 9:29 AM
> To: Danny Gonzalez 
> Cc: openjfx-dev@openjdk.java.net
> Subject: RE: JDK-8177945 : Single cell selection flickers when adding data to
> TableView
>
> This might fix the issue, but I’d like to understand better what the root of 
> the
> problem is. My concern is that this fix might cause a performance regression.
> I’m using the code in  JDK-8177945. I want to look at what TableView does
> when it adds a cell. Is there something about the selected state that
> changes? Or is this just the CSS implementation recalculating styles and un-
> necessarily clearing old values? Some debugging is in order.
>
> From: Danny Gonzalez 
> Sent: Wednesday, March 4, 2020 4:34 AM
> To: David Grieve 
> Cc: openjfx-dev@openjdk.java.net
> Subject: [EXTERNAL] Re: JDK-8177945 : Single cell selection flickers when
> adding data to TableView
>
> Hi David,
>
> Just wanted to add some more information.
>
> The cell selection flashing issue goes away If I put back the following code 
> in
> the layout function in Parent.java:
>
> //
> // One idiom employed by developers is to, during the layout 
> pass,
> // add or remove nodes from the scene. For example, a 
> ScrollPane
> // might add scroll bars to itself if it determines during 
> layout
> // that it needs them, or a ListView might add cells to 
> itself if
> // it determines that it needs to. In such situations we must
> // apply the CSS immediately and not add it to the scene's 
> queue
> // for deferred action.
> //
> // Note that we only call processCSS if the flag is update. 
> If the
> // flag is DIRTY_BRANCH, then the whatever children need 
> UPDATE
> // will be visited during this layout anyway. On the next 
> pulse,
> // doCSSPass will clean up the DIRTY_BRANCH nodes.
> //
> if (cssFlag == CssFlags.UPDATE) {
> processCSS();
> }
>
> This code was originally added in in the following commit:
>
> commit e76b5755d4d2752037cc95eb75cb2615a740cc30
> Author: David Grieve
> mailto:dgri...@openjdk.org>>
> Date:   Thu Apr 10 15:40:34 2014 -0400
>
> RT-36559: Some css optimizations: 1 - on impl_reapplyCSS, do not reapply
> css to child nodes if nothing has changed. 2 - on applyCss, only look for
> ancestor node with css state = UPDATE. 3 - only recalculate checksum of css
> file if the file has been removed from a scene or parent
>
>
> It was reverted out in this commit:
>
> commit 05afad6b528e871d607b76aea2642cf788b417fe
> Author: David Grieve
> mailto:dgri...@openjdk.org>>
> Dat

RE: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-10 Thread David Grieve
The flickering is happening because of the way VirtualFlow reuses cells. The 
selected state gets cleared and reset, which is the flicker. 
This change fixed the flickering for me. I'm not sure why the index of a cell 
was being set to -1, but the effect of doing so is that when the
cell is pulled out of the pile, it looks like a new cell that has to have its 
index set, selected state set, etc, etc. By commenting out the 
updateIndex(-1), VirtualFlow can reuse the cell - setting the index, selected 
state, etc turns into noops (more or less). 

Again, there may have been a good reason for setting index to -1 in this 
condition. More investigation needs to be done. 

diff --git 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
index 728e7c5513..a58859bd05 100644
--- 
a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
+++ 
b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
@@ -991,9 +991,9 @@ public class VirtualFlow extends 
Region {
 lastWidth = -1;
 lastHeight = -1;
 releaseCell(accumCell);
-for (int i = 0, max = cells.size(); i < max; i++) {
-cells.get(i).updateIndex(-1);
-}
+//for (int i = 0, max = cells.size(); i < max; i++) {
+//cells.get(i).updateIndex(-1);
+//}
 addAllToPile();
 releaseAllPrivateCells();
 } else if (needsReconfigureCells) {

> -Original Message-
> From: openjfx-dev  On Behalf Of
> David Grieve
> Sent: Friday, March 6, 2020 9:29 AM
> To: Danny Gonzalez 
> Cc: openjfx-dev@openjdk.java.net
> Subject: RE: JDK-8177945 : Single cell selection flickers when adding data to
> TableView
> 
> This might fix the issue, but I’d like to understand better what the root of 
> the
> problem is. My concern is that this fix might cause a performance regression.
> I’m using the code in  JDK-8177945. I want to look at what TableView does
> when it adds a cell. Is there something about the selected state that
> changes? Or is this just the CSS implementation recalculating styles and un-
> necessarily clearing old values? Some debugging is in order.
> 
> From: Danny Gonzalez 
> Sent: Wednesday, March 4, 2020 4:34 AM
> To: David Grieve 
> Cc: openjfx-dev@openjdk.java.net
> Subject: [EXTERNAL] Re: JDK-8177945 : Single cell selection flickers when
> adding data to TableView
> 
> Hi David,
> 
> Just wanted to add some more information.
> 
> The cell selection flashing issue goes away If I put back the following code 
> in
> the layout function in Parent.java:
> 
> //
> // One idiom employed by developers is to, during the layout 
> pass,
> // add or remove nodes from the scene. For example, a 
> ScrollPane
> // might add scroll bars to itself if it determines during 
> layout
> // that it needs them, or a ListView might add cells to 
> itself if
> // it determines that it needs to. In such situations we must
> // apply the CSS immediately and not add it to the scene's 
> queue
> // for deferred action.
> //
> // Note that we only call processCSS if the flag is update. 
> If the
> // flag is DIRTY_BRANCH, then the whatever children need 
> UPDATE
> // will be visited during this layout anyway. On the next 
> pulse,
> // doCSSPass will clean up the DIRTY_BRANCH nodes.
> //
> if (cssFlag == CssFlags.UPDATE) {
> processCSS();
> }
> 
> This code was originally added in in the following commit:
> 
> commit e76b5755d4d2752037cc95eb75cb2615a740cc30
> Author: David Grieve
> mailto:dgri...@openjdk.org>>
> Date:   Thu Apr 10 15:40:34 2014 -0400
> 
> RT-36559: Some css optimizations: 1 - on impl_reapplyCSS, do not reapply
> css to child nodes if nothing has changed. 2 - on applyCss, only look for
> ancestor node with css state = UPDATE. 3 - only recalculate checksum of css
> file if the file has been removed from a scene or parent
> 
> 
> It was reverted out in this commit:
> 
> commit 05afad6b528e871d607b76aea2642cf788b417fe
> Author: David Grieve
> mailto:dgri...@openjdk.org>>
> Date:   Tue Apr 15 11:51:38 2014 -0400
> 
> RT-36672: move code to process css during layout back to impl_reapplyCSS,
> which is where it was priort to RT-36559
> 
> 
> 
> 
> This was the point at which the cell selection flashing appeared.
> 
&

RE: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-06 Thread David Grieve
This might fix the issue, but I’d like to understand better what the root of 
the problem is. My concern is that this fix might cause a performance 
regression.
I’m using the code in  JDK-8177945. I want to look at what TableView does when 
it adds a cell. Is there something about the selected state that changes? Or is 
this just the CSS implementation recalculating styles and un-necessarily 
clearing old values? Some debugging is in order.

From: Danny Gonzalez 
Sent: Wednesday, March 4, 2020 4:34 AM
To: David Grieve 
Cc: openjfx-dev@openjdk.java.net
Subject: [EXTERNAL] Re: JDK-8177945 : Single cell selection flickers when 
adding data to TableView

Hi David,

Just wanted to add some more information.

The cell selection flashing issue goes away If I put back the following code in 
the layout function in Parent.java:

//
// One idiom employed by developers is to, during the layout 
pass,
// add or remove nodes from the scene. For example, a ScrollPane
// might add scroll bars to itself if it determines during 
layout
// that it needs them, or a ListView might add cells to itself 
if
// it determines that it needs to. In such situations we must
// apply the CSS immediately and not add it to the scene's queue
// for deferred action.
//
// Note that we only call processCSS if the flag is update. If 
the
// flag is DIRTY_BRANCH, then the whatever children need UPDATE
// will be visited during this layout anyway. On the next pulse,
// doCSSPass will clean up the DIRTY_BRANCH nodes.
//
if (cssFlag == CssFlags.UPDATE) {
processCSS();
}

This code was originally added in in the following commit:

commit e76b5755d4d2752037cc95eb75cb2615a740cc30
Author: David Grieve mailto:dgri...@openjdk.org>>
Date:   Thu Apr 10 15:40:34 2014 -0400

RT-36559: Some css optimizations: 1 - on impl_reapplyCSS, do not reapply 
css to child nodes if nothing has changed. 2 - on applyCss, only look for 
ancestor node with css state = UPDATE. 3 - only recalculate checksum of css 
file if the file has been removed from a scene or parent


It was reverted out in this commit:

commit 05afad6b528e871d607b76aea2642cf788b417fe
Author: David Grieve mailto:dgri...@openjdk.org>>
Date:   Tue Apr 15 11:51:38 2014 -0400

RT-36672: move code to process css during layout back to impl_reapplyCSS, 
which is where it was priort to RT-36559




This was the point at which the cell selection flashing appeared.


Thanks


Danny



On 3 Mar 2020, at 16:50, David Grieve 
mailto:david.gri...@microsoft.com>> wrote:

The importance of 05afad6 Is there in the commit itself:

+//
+// One idiom employed by developers is to, during the layout pass,
+// add or remove nodes from the scene. For example, a ScrollPane
+// might add scroll bars to itself if it determines during layout
+// that it needs them, or a ListView might add cells to itself if
+// it determines that it needs to. In such situations we must
+// apply the CSS immediately and not add it to the scene's queue
+// for deferred action.
+

If you revert 05afad6, you'll break this.

This is the first time I've become aware of this flickering issue. I'll have to 
look at it.
I wonder if the fix for 
https://bugs.openjdk.java.net/browse/JDK-8193445<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8193445=02%7C01%7CDavid.Grieve%40microsoft.com%7C9148c860ff524a32ac3208d7c01f2523%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637189112753556661=mZrjldSw%2Fjbd0ita58XMU%2BaKM2GlvObq3aaQmQb5Poc%3D=0>
 has any impact on this.


-Original Message-
From: openjfx-dev 
mailto:openjfx-dev-boun...@openjdk.java.net>>
 On Behalf Of
Danny Gonzalez
Sent: Tuesday, March 3, 2020 11:14 AM
To: openjfx-dev@openjdk.java.net<mailto:openjfx-dev@openjdk.java.net>
Subject: [EXTERNAL] JDK-8177945 : Single cell selection flickers when adding
data to TableView


There is currently an open bug to do with the issue of selection flickering
when using single cell selection and when adding data to a TableView.
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
.java.com%2Fbugdatabase%2Fview_bug.do%3Fbug_id%3D8177945da
ta=02%7C01%7Cdavid.grieve%40microsoft.com%7C91a16d9ab05340719f3008
d7bf8e3410%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63718848
9816399816sdata=wdPRh3VnN%2BfJw%2BatKOyBi9%2Ba2%2FidMJJb
8AwcRXVrfLo%3Dreserved=0

This bug seems to be low priority because it hasn’t been looked at since I
submitted it at the start of 2017.

This is quite a major issue for us so I have narrowed down when the issue
was first introduced.

Here is the changeset:

commit 05afad6b528e871d607b76aea

Re: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-04 Thread Danny Gonzalez
Hi David,

Just wanted to add some more information.

The cell selection flashing issue goes away If I put back the following code in 
the layout function in Parent.java:

//
// One idiom employed by developers is to, during the layout 
pass,
// add or remove nodes from the scene. For example, a ScrollPane
// might add scroll bars to itself if it determines during 
layout
// that it needs them, or a ListView might add cells to itself 
if
// it determines that it needs to. In such situations we must
// apply the CSS immediately and not add it to the scene's queue
// for deferred action.
//
// Note that we only call processCSS if the flag is update. If 
the
// flag is DIRTY_BRANCH, then the whatever children need UPDATE
// will be visited during this layout anyway. On the next pulse,
// doCSSPass will clean up the DIRTY_BRANCH nodes.
//
if (cssFlag == CssFlags.UPDATE) {
processCSS();
}

This code was originally added in in the following commit:

commit e76b5755d4d2752037cc95eb75cb2615a740cc30
Author: David Grieve mailto:dgri...@openjdk.org>>
Date:   Thu Apr 10 15:40:34 2014 -0400

RT-36559: Some css optimizations: 1 - on impl_reapplyCSS, do not reapply 
css to child nodes if nothing has changed. 2 - on applyCss, only look for 
ancestor node with css state = UPDATE. 3 - only recalculate checksum of css 
file if the file has been removed from a scene or parent


It was reverted out in this commit:

commit 05afad6b528e871d607b76aea2642cf788b417fe
Author: David Grieve mailto:dgri...@openjdk.org>>
Date:   Tue Apr 15 11:51:38 2014 -0400

RT-36672: move code to process css during layout back to impl_reapplyCSS, 
which is where it was priort to RT-36559


This was the point at which the cell selection flashing appeared.

Thanks

Danny


On 3 Mar 2020, at 16:50, David Grieve 
mailto:david.gri...@microsoft.com>> wrote:

The importance of 05afad6 Is there in the commit itself:

+//
+// One idiom employed by developers is to, during the layout pass,
+// add or remove nodes from the scene. For example, a ScrollPane
+// might add scroll bars to itself if it determines during layout
+// that it needs them, or a ListView might add cells to itself if
+// it determines that it needs to. In such situations we must
+// apply the CSS immediately and not add it to the scene's queue
+// for deferred action.
+

If you revert 05afad6, you'll break this.

This is the first time I've become aware of this flickering issue. I'll have to 
look at it.
I wonder if the fix for https://bugs.openjdk.java.net/browse/JDK-8193445 has 
any impact on this.

-Original Message-
From: openjfx-dev 
mailto:openjfx-dev-boun...@openjdk.java.net>>
 On Behalf Of
Danny Gonzalez
Sent: Tuesday, March 3, 2020 11:14 AM
To: openjfx-dev@openjdk.java.net
Subject: [EXTERNAL] JDK-8177945 : Single cell selection flickers when adding
data to TableView


There is currently an open bug to do with the issue of selection flickering
when using single cell selection and when adding data to a TableView.
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
.java.com%2Fbugdatabase%2Fview_bug.do%3Fbug_id%3D8177945da
ta=02%7C01%7Cdavid.grieve%40microsoft.com%7C91a16d9ab05340719f3008
d7bf8e3410%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63718848
9816399816sdata=wdPRh3VnN%2BfJw%2BatKOyBi9%2Ba2%2FidMJJb
8AwcRXVrfLo%3Dreserved=0

This bug seems to be low priority because it hasn’t been looked at since I
submitted it at the start of 2017.

This is quite a major issue for us so I have narrowed down when the issue
was first introduced.

Here is the changeset:

commit 05afad6b528e871d607b76aea2642cf788b417fe
Author: David Grieve
mailto:dgri...@openjdk.org>>
Date:   Tue Apr 15 11:51:38 2014 -0400

   RT-36672: move code to process css during layout back to impl_reapplyCSS,
which is where it was priort to RT-36559


I can’t search the bug database for this bug ID as I believe it was submitted
when a previous bug tracking system was in use.

Does anyone have any knowledge as to why this fix was needed and what
the repercussions would be if I reverted it out for our local OpenJFX build.

Alternatively I would be glad to receive any suggestions of how to fix the
flickering issue if this changeset is important to leave in.

Thanks

Danny



Re: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-03 Thread Danny Gonzalez
Thanks for taking a look at this David.

Danny

> On 3 Mar 2020, at 16:50, David Grieve  wrote:
> 
> The importance of 05afad6 Is there in the commit itself: 
> 
> +//
> +// One idiom employed by developers is to, during the layout pass,
> +// add or remove nodes from the scene. For example, a ScrollPane
> +// might add scroll bars to itself if it determines during layout
> +// that it needs them, or a ListView might add cells to itself if
> +// it determines that it needs to. In such situations we must
> +// apply the CSS immediately and not add it to the scene's queue
> +// for deferred action.
> +
> 
> If you revert 05afad6, you'll break this. 
> 
> This is the first time I've become aware of this flickering issue. I'll have 
> to look at it. 
> I wonder if the fix for https://bugs.openjdk.java.net/browse/JDK-8193445 has 
> any impact on this.  
> 
>> -Original Message-
>> From: openjfx-dev  On Behalf Of
>> Danny Gonzalez
>> Sent: Tuesday, March 3, 2020 11:14 AM
>> To: openjfx-dev@openjdk.java.net
>> Subject: [EXTERNAL] JDK-8177945 : Single cell selection flickers when adding
>> data to TableView
>> 
>> 
>> There is currently an open bug to do with the issue of selection flickering
>> when using single cell selection and when adding data to a TableView.
>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
>> .java.com%2Fbugdatabase%2Fview_bug.do%3Fbug_id%3D8177945da
>> ta=02%7C01%7Cdavid.grieve%40microsoft.com%7C91a16d9ab05340719f3008
>> d7bf8e3410%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63718848
>> 9816399816sdata=wdPRh3VnN%2BfJw%2BatKOyBi9%2Ba2%2FidMJJb
>> 8AwcRXVrfLo%3Dreserved=0
>> 
>> This bug seems to be low priority because it hasn’t been looked at since I
>> submitted it at the start of 2017.
>> 
>> This is quite a major issue for us so I have narrowed down when the issue
>> was first introduced.
>> 
>> Here is the changeset:
>> 
>> commit 05afad6b528e871d607b76aea2642cf788b417fe
>> Author: David Grieve
>> mailto:dgri...@openjdk.org>>
>> Date:   Tue Apr 15 11:51:38 2014 -0400
>> 
>>RT-36672: move code to process css during layout back to impl_reapplyCSS,
>> which is where it was priort to RT-36559
>> 
>> 
>> I can’t search the bug database for this bug ID as I believe it was submitted
>> when a previous bug tracking system was in use.
>> 
>> Does anyone have any knowledge as to why this fix was needed and what
>> the repercussions would be if I reverted it out for our local OpenJFX build.
>> 
>> Alternatively I would be glad to receive any suggestions of how to fix the
>> flickering issue if this changeset is important to leave in.
>> 
>> Thanks
>> 
>> Danny



RE: JDK-8177945 : Single cell selection flickers when adding data to TableView

2020-03-03 Thread David Grieve
The importance of 05afad6 Is there in the commit itself: 

+//
+// One idiom employed by developers is to, during the layout pass,
+// add or remove nodes from the scene. For example, a ScrollPane
+// might add scroll bars to itself if it determines during layout
+// that it needs them, or a ListView might add cells to itself if
+// it determines that it needs to. In such situations we must
+// apply the CSS immediately and not add it to the scene's queue
+// for deferred action.
+

If you revert 05afad6, you'll break this. 

This is the first time I've become aware of this flickering issue. I'll have to 
look at it. 
I wonder if the fix for https://bugs.openjdk.java.net/browse/JDK-8193445 has 
any impact on this.  

> -Original Message-
> From: openjfx-dev  On Behalf Of
> Danny Gonzalez
> Sent: Tuesday, March 3, 2020 11:14 AM
> To: openjfx-dev@openjdk.java.net
> Subject: [EXTERNAL] JDK-8177945 : Single cell selection flickers when adding
> data to TableView
> 
> 
> There is currently an open bug to do with the issue of selection flickering
> when using single cell selection and when adding data to a TableView.
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .java.com%2Fbugdatabase%2Fview_bug.do%3Fbug_id%3D8177945da
> ta=02%7C01%7Cdavid.grieve%40microsoft.com%7C91a16d9ab05340719f3008
> d7bf8e3410%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63718848
> 9816399816sdata=wdPRh3VnN%2BfJw%2BatKOyBi9%2Ba2%2FidMJJb
> 8AwcRXVrfLo%3Dreserved=0
> 
> This bug seems to be low priority because it hasn’t been looked at since I
> submitted it at the start of 2017.
> 
> This is quite a major issue for us so I have narrowed down when the issue
> was first introduced.
> 
> Here is the changeset:
> 
> commit 05afad6b528e871d607b76aea2642cf788b417fe
> Author: David Grieve
> mailto:dgri...@openjdk.org>>
> Date:   Tue Apr 15 11:51:38 2014 -0400
> 
> RT-36672: move code to process css during layout back to impl_reapplyCSS,
> which is where it was priort to RT-36559
> 
> 
> I can’t search the bug database for this bug ID as I believe it was submitted
> when a previous bug tracking system was in use.
> 
> Does anyone have any knowledge as to why this fix was needed and what
> the repercussions would be if I reverted it out for our local OpenJFX build.
> 
> Alternatively I would be glad to receive any suggestions of how to fix the
> flickering issue if this changeset is important to leave in.
> 
> Thanks
> 
> Danny