On Wed, 9 Oct 2024 20:15:19 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line >> 35: >> >>> 33: * <p> >>> 34: * The snapshot is valid until the layout changes due to any change that >>> 35: * triggers that, such as resizing of the container or modification of >>> properties. >> >> "until the layout changes due to any change that triggers that" >> >> it's not wrong, but writing change twice reads a bit off. >> How about >> >> The layout snapshot is no longer valid after actions such as resizing of the >> container, or modification of certain properties. >> For example updating the text or the font would invalidate the layout >> snapshot, but a change of color would not. >> >> >> I still don't see how applications can KNOW this has happened. >> There's nothing on the (immutable) layout which says its invalidated. >> There's no property to monitor. >> So people will in effect never be able to do anything but use it and toss it >> and get a new one every time. >> Perhaps you can make it 'cheap' to do this. >> Since it is immutable, the Text or TextFlow can cache it. >> If it is invalidated, the Text or TextFlow should know and can flush its >> cached version. >> So the SAME immutable instance can be returned over and over again until >> that happens. >> >> People can then also use "==" or equals() to check this and save themselves >> validation too. >> I notice you don't have equals() so probably that is needed anyway to save >> people from manual comparison .. also an evolution issue if the class is >> updated. >> >> A consequence of that is then the text about invalidation can be updated >> with advice on doing a simple equals() call if they need to know if it >> changed. And equals will start with "==" so will be really fast to return >> true .. > > thank you! > > I am struggling with how to explain that it should neither be cached, nor > compared to another snapshot. > > Maybe an example would help. Use case is an editable rich text component > based on `TextFlow`. The user presses `HOME` and the caret needs to go to > the beginning of the line. We get the `LayoutInfo`, determine which line the > caret is currently at and determine the start offset of that line to move the > caret to. > > Another example: in `Text`, to determine whether the mouse is clicked inside > of the text or beyond the last character (see > [JDK-8091012](https://bugs.openjdk.org/browse/JDK-8091012) ). > > We are not expecting the application to monitor the text layout, so no > notifications or observable properties or equals() are needed. > > (I've added some words to that effect in `Text/TextFlow.getLayoutInfo()` ) Clients will usually obtain a LayoutInfo in order to respond to some user action or an update of the target node. LayoutInfo should be considered an ephemeral snapshot, to be used and immediately discarded. Clients should always obtain a new one when they need to respond again as likely the trigger to respond again means the target node has changed. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1794216169
