RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]

[John Hendrikx]

On Mon, 19 Feb 2024 13:45:00 GMT, Karthik P K <kpk at openjdk.org> wrote:

> > @karthikpandelu You mentioned there is special casing going on when a `Text` is part of a `TextFlow`. What are those cases and where is this happening? How does it even know that a `Text` is involved and that it is part of a `TextFlow`?
> 
> We see these cases for every case when Text node is embedded in TextFlow. Basically we do not get x coordinate relative to the Text node since it is embedded in TextFlow. There are additional scenarios like RichText and Inline nodes present in TextFlow as well. In this case spans != null and text is supplied as parameter from Text class. This condition is considered as Text embedded in TextFlow. Below is the simple test code which shows above mentioned case.

Okay, so let me summarize (and let me know if that's correct):

- You have a `Text` embedded into `TextFlow`
- Mouse supplies a coordinate (`PickResult`)
- You call `hitTest` on the node under the mouse (the intersected node, in this case, a `Text`)
- The `Text` notices it is part of a `TextFlow`, and uses the parents `TextLayout` (which uses different coordinates)
- We make the mouse coordinates relative to the `Text` node (not to the `TextFlow`, even though it uses its `TextLayout`...)
- It then proceeds to call `layout.getHitInfo` with the `x`, `y` coordinates which are relative to the `Text`, but to compensate for that provides runStart/runEnd + extra logic in `getHitInfo`

So, where I think there is a disconnect here between what I would expect and what is being implemented here is that the logic to compensate for a `Text` being part of a `TextFlow` has been pushed down into `PrismTextLayout` -- to compensate properly, `getHitInfo` needs a lot of extra parameters and has special paths for cases where a `Text` is part of a `TextFlow`.

To determine the runStart/runEnd, `Text` is already analyzing the runs (and doing coordinate tests on them).  Runs however have locations, with x/y coordinates.  Would it not be possible, and more sensible, to adjust the `x`, `y` coordinates passed to `getHitInfo` instead of leaving those as coordinates relative to the `Text` ? I mean, you are using the parents `TextLayout`, so it would make sense to pass that `TextLayout` coordinates that are correct for it in the first place?

In other words, I think the last few steps should be:

- The `Text` notices it is part of a `TextFlow`, and uses the parents `TextLayout`
- If using the parent `TextLayout`, then coordinates are made relative to the `TextFlow` node by analyzing the run locations
- It then proceeds to call `layout.getHitInfo` with the new `x`, `y` coordinates which are relative to the `TextFlow`

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1952572682