[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: Virtual Flow enhancements
From: Dirk Lemmermann <dlemmermann () gmail ! com>
Date: 2023-01-28 14:25:36
Message-ID: 48F14931-D88B-4847-BF52-3866C282C3AF () gmail ! com
[Download RAW message or body]
My customer PSI has now also created their own fork and implemented a mechanism to \
specify row heights explicitly:
"https://github.com/PSI-Polska/jfx17u/tree/jfx-17-PSI - look at 3 commits with \
"8277380", workaround for JDK-8277380. FlexGantt provides the row sizes, so it wasn't \
hard to adopt it to use row size/height supplier."
— Dirk
> On 22 Nov 2022, at 12:13, Johan Vos <johan.vos@gluonhq.com> wrote:
>
> That is good food indeed. It would be sort of a circumvention of what is often \
> leading to major issues of different kinds: the Cell.updateItem is used for a \
> number of differen goals. The javadoc for this one [1] is already an indication \
> that this is something dangerous:"It is very important that subclasses of Cell \
> override the updateItem method properly".
> Even when doing it properly (and granted, there is no clear definition of \
> "properly" here), there are still different usecases. In case the method is called \
> because the specific item is about to be rendered in that specific cell, it makes \
> sense that there is often additional processing needed. However, that same method \
> is also called when the VirtualFlow needs to get the boundaries of the specified \
> item when it would be rendered in a cell. Often, these two cases come down to the \
> same: if we want the know the dimensions of something, we can populate the cell and \
> then return its dimensions. This can lead to significant overhead:
> 1. you know ahead of time what the size of a specific cell is (in which case the \
> suggestion from Dirk is valid), so no need for any computation 2. you need to do \
> additional processing in case the cell is really rendered.
> This can sort of being bypassed by checking if the current cell is the accumCell, \
> but this is not a documented API. I personally think we can do a bit more with the \
> accumCell, which is used internally by the VirtualFlow when we need to do \
> operations on a cell that is not (inteded to be) visible. If the invocation of \
> `Cell.updateItem` makes it crystal clear if we are invoking this method either to \
> (really draw the item in the cell and render it) or (to do dimension calculations) \
> developers have an easier way to pass dimension information.
> - Johan
>
> [1] https://openjfx.io/javadoc/19/javafx.controls/javafx/scene/control/Cell.html#updateItem(T,boolean)
>
> On Tue, Nov 22, 2022 at 11:44 AM Dirk Lemmermann <dlemmermann@gmail.com \
> <mailto:dlemmermann@gmail.com>> wrote:
> > Food for thought: something that might be nice to have could be a separate model \
> > that tells the VirtualFlows what the row heights are. In FlexGanttFX the height \
> > of each row is explicitly controlled by a heightProperty() of a "row" class and \
> > not by calculation. E.g. VirtualFlow.setHeightProvider(...) or (to support \
> > vertical and horizontal flows) VirtualFlow.setSizeProvider(…). This could then \
> > be used to pre-populate the size cache. If your application stays the in the \
> > range of hundreds of rows this could work pretty fast.
> > > On 28 Oct 2022, at 14:43, Dirk Lemmermann <dlemmermann@gmail.com \
> > > <mailto:dlemmermann@gmail.com>> wrote:
> > > Looks like I missed this last replay from Johan. In my last email I was \
> > > referring to a work-around where one VirtualFlow gets repositioned via \
> > > scrollTo() method calls in response to changes in the other VirtualFlow. Not \
> > > only are the rows alignments way off but updates are lagging behind.
> > > But let's leave that behind for now and let's try to find an existing property \
> > > that would make our use-case work again.
> > > For the "FlexGanttFX" use-case where I need to syncronize the scrolling of a \
> > > TreeTableView and a ListView I would love to be able to simply bind the \
> > > "position" properties of those two controls with each other. That feels very \
> > > intuitive to me. If both controls have the same number of rows and each row's \
> > > height matches the row's height in the other control then this should work, but \
> > > currently it does not.
> > > The "position" property gets updated by the VirtualFlow. When the flow sets \
> > > this property to a certain value then I would hope setting the same value from \
> > > outside would place the virtual flow at the exact same position. Basically I am \
> > > hoping that this is a bijective mapping but it is not …. unless … the user \
> > > scrolled all the way down in both views. Then it becomes a bijective mapping. \
> > > So I guess after having made all rows visible the calculations are based on \
> > > hard facts (as in "actual row height") and not on estimates.
> > > To summarise the requirement: there should be a way to bind a property of \
> > > VirtualFlow so that two instances with the same content can be scrolled in sync \
> > > (content = same rows with same heights resulting in same total virtual height). \
> > > Dirk
> > >
> > > > On 15 Sep 2022, at 21:20, Johan Vos <johan.vos@gluonhq.com \
> > > > <mailto:johan.vos@gluonhq.com>> wrote:
> > > >
> > > >
> > > > On Wed, Sep 14, 2022 at 12:19 PM Dirk Lemmermann <dlemmermann@gmail.com \
> > > > <mailto:dlemmermann@gmail.com>> wrote:
> > > > > Hi,
> > > > >
> > > > >
> > > > > FlexGanttFX used to make this work via bidirectional bindings of the \
> > > > > properties of the vertical scrollbars of both VirtualFlows. With the latest \
> > > > > fixes to the VirtualFlow the assumption that two identically populated \
> > > > > VirtualFlows would provide identical values to the ScrollBar properties is \
> > > > > no longer true. The attempt to bind the "position" property also failed and \
> > > > > a work-around that Johan provided also has not been successful, yet (a \
> > > > > customer of mine is still evaluating it).
> > > >
> > > > I don't know what work-around you refer to, but I often point to public \
> > > > methods in VirtualFlow that, when properly combined, allow many usecases. I \
> > > > sometimes see code where the information about the positioning of elements in \
> > > > the VirtualFlow is obtained via the position of the scrollbar thumb, which \
> > > > seems a really odd way to get this info (and especially unreliable as the \
> > > > relation with the real positioning of cells is unspecified). There are other \
> > > > methods on VirtualFlow that imho are better suited for getting/setting \
> > > > information. What I want to avoid is that we have 2 API's that almost \
> > > > achieve the same. Hence, before considering a new method or property, I think \
> > > > we should make sure that there is currently no existing (documented) way to \
> > > > achieve it. I am pretty sure there are cases that can not be solved with the \
> > > > existing set of API's, and those cases are exactly what I'm looking for.
> > > > - Johan
> > > >
> > > >
> > >
> >
[Attachment #3 (unknown)]
<html><head><meta http-equiv="content-type" content="text/html; \
charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: \
space; line-break: after-white-space;">My customer PSI has now also created their own \
fork and implemented a mechanism to specify row heights \
explicitly:<div><br></div><div>"<a \
href="https://github.com/PSI-Polska/jfx17u/tree/jfx-17-PSI" style="box-sizing: \
border-box; color: var(--color-accent-fg); text-decoration: none; font-family: \
-apple-system, BlinkMacSystemFont, "Segoe UI", "Noto Sans", \
Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI \
Emoji";">https://github.com/PSI-Polska/jfx17u/tree/jfx-17-PSI</a><span \
style="caret-color: rgb(36, 41, 47); color: rgb(36, 41, 47); font-family: \
-apple-system, BlinkMacSystemFont, "Segoe UI", "Noto Sans", \
Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI \
Emoji"; background-color: rgb(255, 255, 255);"> - look at 3 commits with \
"8277380", workaround for JDK-8277380. FlexGantt provides the row sizes, so it wasn't \
hard to adopt it to use row size/height supplier.</span><font color="#24292f" \
face="-apple-system, BlinkMacSystemFont, Segoe UI, Noto Sans, Helvetica, Arial, \
sans-serif, Apple Color Emoji, Segoe UI Emoji"><span style="caret-color: rgb(36, 41, \
47);">"</span></font></div><div><span style="caret-color: rgb(36, 41, 47); color: \
rgb(36, 41, 47); font-family: -apple-system, BlinkMacSystemFont, "Segoe \
UI", "Noto Sans", Helvetica, Arial, sans-serif, "Apple Color \
Emoji", "Segoe UI Emoji"; background-color: rgb(255, 255, \
255);"><br></span></div><div><span style="background-color: rgb(255, 255, \
255);"><font color="#24292f" face="-apple-system, BlinkMacSystemFont, Segoe UI, Noto \
Sans, Helvetica, Arial, sans-serif, Apple Color Emoji, Segoe UI Emoji"><span \
style="caret-color: rgb(36, 41, \
47);">— Dirk</span></font></span></div><div><br></div><div><div><br><blockquote \
type="cite"><div>On 22 Nov 2022, at 12:13, Johan Vos <johan.vos@gluonhq.com> \
wrote:</div><br class="Apple-interchange-newline"><div><div dir="ltr">That is good \
food indeed. It would be sort of a circumvention of what is often leading to major \
issues of different kinds: the Cell.updateItem is used for a number of differen \
goals. The javadoc for this one [1] is already an indication that this is something \
dangerous:"It is very important that subclasses of Cell override the updateItem \
method properly".<div><br><div>Even when doing it properly (and granted, there is no \
clear definition of "properly" here), there are still different usecases. In case the \
method is called because the specific item is about to be rendered in that specific \
cell, it makes sense that there is often additional processing \
needed. </div><div>However, that same method is also called when the VirtualFlow \
needs to get the boundaries of the specified item when it would be rendered in a \
cell. Often, these two cases come down to the same: if we want the know the \
dimensions of something, we can populate the cell and then return its \
dimensions. </div><div>This can lead to significant overhead:</div><div>1. you \
know ahead of time what the size of a specific cell is (in which case the suggestion \
from Dirk is valid), so no need for any computation</div><div>2. you need to do \
additional processing in case the cell is really \
rendered. </div><div><br></div><div>This can sort of being bypassed by checking \
if the current cell is the accumCell, but this is not a documented API. I personally \
think we can do a bit more with the accumCell, which is used internally by the \
VirtualFlow when we need to do operations on a cell that is not (inteded to be) \
visible. If the invocation of `Cell.updateItem` makes it crystal clear if we are \
invoking this method either to (really draw the item in the cell and render it) or \
(to do dimension calculations) developers have an easier way to pass dimension \
information.</div><div><br></div><div>- Johan <br> <div><div>[1] <a \
href="https://openjfx.io/javadoc/19/javafx.controls/javafx/scene/control/Cell.html#upd \
ateItem(T,boolean)">https://openjfx.io/javadoc/19/javafx.controls/javafx/scene/control/Cell.html#updateItem(T,boolean)</a><br></div></div></div></div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 22, 2022 at 11:44 \
AM Dirk Lemmermann <<a \
href="mailto:dlemmermann@gmail.com">dlemmermann@gmail.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Food for thought: \
something that might be nice to have could be a separate model that tells the \
VirtualFlows what the row heights are. In FlexGanttFX the height of each row is \
explicitly controlled by a heightProperty() of a "row" class and not by calculation. \
E.g. VirtualFlow.setHeightProvider(...) or (to support vertical and horizontal flows) \
VirtualFlow.setSizeProvider(…). This could then be used to pre-populate the size \
cache. If your application stays the in the range of hundreds of rows this could work \
pretty fast. <br><div><br><blockquote type="cite"><div>On 28 Oct 2022, at 14:43, \
Dirk Lemmermann <<a href="mailto:dlemmermann@gmail.com" \
target="_blank">dlemmermann@gmail.com</a>> wrote:</div><br><div><div \
style="overflow-wrap: break-word;">Looks like I missed this last replay from Johan. \
In my last email I was referring to a work-around where one VirtualFlow gets \
repositioned via scrollTo() method calls in response to changes in the other \
VirtualFlow. Not only are the rows alignments way off but updates are lagging \
behind.<div><br></div><div>But let's leave that behind for now and let's try to find \
an existing property that would make our use-case work \
again.<br><div><br></div><div>For the "FlexGanttFX" use-case where I need to \
syncronize the scrolling of a TreeTableView and a ListView I would love to be \
able to simply bind the "position" properties of those two controls with each other. \
That feels very intuitive to me. If both controls have the same number of rows and \
each row's height matches the row's height in the other control then this should \
work, but currently it does not. </div><div><br></div><div>The "position" \
property gets updated by the VirtualFlow. When the flow sets this property to a \
certain value then I would hope setting the same value from outside would place the \
virtual flow at the exact same position. Basically I am hoping that this is a \
bijective mapping but it is not …. unless … the user scrolled all the way down in \
both views. Then it becomes a bijective mapping. So I guess after having made all \
rows visible the calculations are based on hard facts (as in "actual row height") and \
not on estimates.</div><div><br></div><div>To summarise the requirement: there should \
be a way to bind a property of VirtualFlow so that two instances with the same \
content can be scrolled in sync (content = same rows with same heights resulting in \
same total virtual height).</div><div><br></div><div>Dirk</div></div><div><br><blockquote \
type="cite"><div>On 15 Sep 2022, at 21:20, Johan Vos <<a \
href="mailto:johan.vos@gluonhq.com" target="_blank">johan.vos@gluonhq.com</a>> \
wrote:</div><br><div><div dir="ltr"><div dir="ltr"><br></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 14, 2022 at 12:19 \
PM Dirk Lemmermann <<a href="mailto:dlemmermann@gmail.com" \
target="_blank">dlemmermann@gmail.com</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex">Hi,<br><br> <br>
FlexGanttFX used to make this work via bidirectional bindings of the properties of \
the vertical scrollbars of both VirtualFlows. With the latest fixes to the \
VirtualFlow the assumption that two identically populated VirtualFlows would provide \
identical values to the ScrollBar properties is no longer true. The attempt to bind \
the "position" property also failed and a work-around that Johan provided also has \
not been successful, yet (a customer of mine is still evaluating \
it).<br></blockquote><div><br></div><div>I don't know what work-around you refer to, \
but I often point to public methods in VirtualFlow that, when properly combined, \
allow many usecases. I sometimes see code where the information about the \
positioning of elements in the VirtualFlow is obtained via the position of the \
scrollbar thumb, which seems a really odd way to get this info (and especially \
unreliable as the relation with the real positioning of cells is unspecified). There \
are other methods on VirtualFlow that imho are better suited for getting/setting \
information. </div><div>What I want to avoid is that we have 2 API's that almost \
achieve the same. Hence, before considering a new method or property, I think we \
should make sure that there is currently no existing (documented) way to achieve it. \
I am pretty sure there are cases that can not be solved with the existing set of \
API's, and those cases are exactly what I'm looking \
for.<br></div><div><br></div><div>- \
Johan</div><div><br></div><div><br></div></div></div> \
</div></blockquote></div><br></div></div></blockquote></div><br></div></blockquote></div>
</div></blockquote></div><br></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic