Re: RFR: 8185887: TableRowSkinBase fails to correctly virtualize cells in horizontal direction [v4]

[yosbits]

On Wed, 9 Sep 2020 02:46:37 GMT, yosbits 
 wrote:

>> @yososs Per [this 
>> message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html)
>>  on the
>> openjfx-dev mailing list, I have closed 
>> [JDK-8185886](https://bugs.openjdk.java.net/browse/JDK-8185886) as a 
>> duplicate,
>> and suggested another existing JBS issue for this PR to use. Please change 
>> the title to:  8185887: TableRowSkinBase
>> fails to correctly virtualize cells in horizontal direction
>
> Below are the answers to JBS's suggestions.
> 
>> The patch below resolves the issue, but it is not likely the correct 
>> solution (we are trying to determine the table
>> width, but we are getting the width and padding on the underlying 
>> virtualflow (the width is fine, the padding should
>> really come from tableview directly):
> 
> You probably mention this part,
> At least padding refers to Skinnable(TableRow or TreeTableRow). This is the 
> same as before (L683). The width of
> VirtualFlow is determined by the header of Table.
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java#L360-L365
>  Java
>  final VirtualFlow virtualFlow = getVirtualFlow();
>  final double scrollX = virtualFlow == null ? 0.0 : 
> virtualFlow.getHbar().getValue();
>  final Insets padding = getSkinnable().getPadding();
>  final double vfWidth = virtualFlow == null ? 0.0:virtualFlow.getWidth();
>  final double headerWidth = vfWidth - (padding.getLeft() + 
> padding.getRight());
> 
> For example, can you assume a case that does not work properly?

This is the answer to JBS's comment.

The BigTreeTableViewTest.java attached to this PR commentary is a modified 
version of the JDK-8166956 test code. The
original test code is good for reproducing the problem, but I have decided that 
it cannot be used as a test code
because the cell values are random and the cell values change randomly with the 
close / expand operation.

-

PR: https://git.openjdk.java.net/jfx/pull/125

Re: RFR: 8185887: TableRowSkinBase fails to correctly virtualize cells in horizontal direction [v4]

[yosbits]

On Tue, 8 Sep 2020 20:19:13 GMT, Kevin Rushforth  wrote:

>> Column virtualization causes shortness of breath when scrolling a large 
>> stroke horizontally.
>> This does not happen when stroking on the scrollbar. Looks like a potential 
>> problem with VirtualFlow.
>> 
>> If you are worried about shortness of breath, the following code will help 
>> reduce the problem.
>> 
>> 
>>  Java
>> 　private static final int OVERLAP_MARGIN = 500;
>> 
>> private static boolean isOverlap(double start, double end, double 
>> start2, double end2){
>>  start = Math.max(start-OVERLAP_MARGIN, start2);
>>  end = Math.min(end+OVERLAP_MARGIN, end2);
>> return (start<=end2 && end >= start2);
>> }
>
> @yososs Per [this 
> message](https://mail.openjdk.java.net/pipermail/openjfx-dev/2020-September/027534.html)
>  on the
> openjfx-dev mailing list, I have closed 
> [JDK-8185886](https://bugs.openjdk.java.net/browse/JDK-8185886) as a 
> duplicate,
> and suggested another existing JBS issue for this PR to use. Please change 
> the title to:  8185887: TableRowSkinBase
> fails to correctly virtualize cells in horizontal direction

Below are the answers to JBS's suggestions.

> The patch below resolves the issue, but it is not likely the correct solution 
> (we are trying to determine the table
> width, but we are getting the width and padding on the underlying virtualflow 
> (the width is fine, the padding should
> really come from tableview directly):

You probably mention this part,
At least padding refers to Skinnable(TableView or TreeTableView). This is the 
same as before (L683). The width of
VirtualFlow is determined by the header of Table.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java#L360-L365
 Java
 final VirtualFlow virtualFlow = getVirtualFlow();
 final double scrollX = virtualFlow == null ? 0.0 : 
virtualFlow.getHbar().getValue();
 final Insets padding = getSkinnable().getPadding();
 final double vfWidth = virtualFlow == null ? 0.0:virtualFlow.getWidth();
 final double headerWidth = vfWidth - (padding.getLeft() + padding.getRight());

For example, can you assume a case that does not work properly?

-

PR: https://git.openjdk.java.net/jfx/pull/125