RFR: 8319844 : Text/TextFlow.hitTest() is incorrect in RTL orientation [v11]
John Hendrikx
jhendrikx at openjdk.org
Thu Feb 15 11:23:06 UTC 2024
On Thu, 15 Feb 2024 10:36:51 GMT, Karthik P K <kpk at openjdk.org> wrote:
> > I think we should simplify the `getHitInfo` method in the `TextLayout` interface.
> > The code currently seems to be making distinctions where there are none. `TextFlow`s provide spans, while `Text`s provide a text. `getHitInfo` can take advantage of this to avoid the `text` and `forTextFlow` parameters altogether. This will reduce confusion (as there is no distinction) and also avoids cases that are non-sensical (providing `text` while `forTextFlow` is `true` or vice versa).
> > Previous changes to this code (when the parameter `text` was introduced to `getHitInfo`) should probably be partially undone and simplified with this new knowledge.
>
> Thanks for reviewing @hjohn Certainly we could simplify the `getHitInfo` method but the complication starts when we have to support Text node embedded in TextFlow. Just to differentiate Text node and TextFlow, spans can be used as you suggested. I had to introduce these parameters for the case of Text node embedded in TextFlow. On top of that we need to support emojis and RTL text. This is where it started getting complex and I had to use these new parameters. If you have any thoughts on this, please let me know. I'll got through all the comments and incorporate wherever possible.
Sure, if you can give me more details as to why this is necessary, perhaps I can follow the logic. From an outside observer, `PrismTextLayout` already seems to have far too much specialization related to how text is supplied (spans/String) and who is supplying it (TextFlow/Text), making the logic hard to follow (but also error prone, and likely has more than a few bugs). I'd expect a class that does text layout measurements to be able to do this with a uniform format without special casing (ie. perhaps `Text` should wrap their text in a `TextSpan` and all the special casing can disappear).
Your current changes seems to increase this divide with even more specialization, hence why I'm wondering if this is the best approach.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1945892410
More information about the openjfx-dev
mailing list