macOS text input #1378

lord · 2020-11-07T00:57:31Z

This change proposes a new set of APIs for text input, as well as fully implements proper text input using those APIs on macOS. Other platforms have text input commands simulated from key down events — this won't handle IME input on those platforms, but will hopefully work as a temporary solution for those platforms until we can add IME support to them.

To test it out: cd druid-shell; cargo run --example edit_text

Sorry for the really long commit!! Let me know if I should split it up. Most of the changes should hopefully be boilerplate and comments, but there's still a fair amount of logic.

I have a few things I'm still unsure about, although I'm sure there's plenty more I've missed:

this i think will allow for RTL/LTR, but do we ever want specially handled vertical text? i guess the macOS APIs don't really seem to handle it specifically, so maybe OK to ignore for now
have not spent too much time considering whether this design works with async functions on android. i've also only barely looked into non-mac platforms for this, but the similarity between this design and tcw3's design makes me feel confident that it'll work on at least gtk and windows, which I think tcw3 has a full text input implementation for.
this is a pretty complex interface! if folks have ideas for how it could be simplified, could be a good idea to try?
are we happy with the taxonomy of TextInputActions I've created? it seems maybe better to me than a completely flat list of commands in a single enum, but perhaps I've over-combined commands, like how vertical movement is shared between selection movements and scrolling. these actions are also a little macOS specific, although I've tried my best to directly implement some of the more esoteric commands like "transpose". tcw3 handles this by having a bunch of post-ime keybindings on other platforms that trigger the appropriate TextInputAction.
not too happy with all the TextInput prefixes everywhere on basic types, but was necessary given how druid-shell puts everything in a single top-level namespace. if we think it's ok to have a text_input_types prefix or something like that, could make types like TextSelection and TextInputUpdate look a little prettier?
this does not yet implement killing and yanking, triggered by control-k and control-y on macOS. hopefully this is ok to defer for now?
there are no tests right now. is this ok? i do think in the long term, would be nice to have a standard test suite that's run on all TextInputHandler implementations to ensure they correctly manipulate the text state, deal with graphemes correctly, etc

See also the IME tracking issue: #1308

cmyr · 2020-11-13T14:58:13Z

Looks like you're plugging away at this but if you think it needs a look or you have any other concerns I'm happy to do a review or have a chat.

lord · 2020-11-14T01:09:56Z

thanks! maybe ~a day more of work and it'll be ready for a first look?

lord · 2020-11-14T20:08:15Z

@cmyr added some more docs, and squashed a few bugs! want to take a peek?

although I've already written a bunch of code, still happy to make bigger structural changes to how the API works if we think it's a good idea!

cmyr

This is really impressive rob, and it definitely stretches my own understanding of the cocoa text system.

To address your specific questions first:

vertical text: happy to put this on the "things to worry about in 2023" list.
async functions on android: not sure how to think about this, but it feels ambitious, and it also doesn't need to be a major concern right now. The fact that there is prior-art for an API like this working on gtk & windows is very compelling, and enough to satisfy my general cross-platform concerns.
complexity: I really don't see many places where this could be simplified, and I actually feel encouraged by its relative simplicity; this is going to be an API that is principally used internally, and given that I think it's pretty good? I'll have to actually see what's involved in making this work with druid proper, but I'm pretty encouraged.
taxonomy: I think this is excellent, and very similar to how we did this in xi. I think sharing things like movement between selection and cursor movement makes total sense.
TextInput prefixes: I would not be opposed to having a text module in druid-shell; that's how we ended up handling it in druid (and a lot of the druid types will end up being replaced by these new types, I think).
kill/yank: This is... I mean, these two (and the various setMark: commands) are just such a pain. They really don't make sense cross-platform. we could definitely consider maintaining a per-app killring in druid-shell, and then implementing these ourselves, and I don't think this would really be that hard, but I think it's okay if this doesn't work for now.
no tests for now is okay. We will need to think about this "eventually", but... 😈

And then other things:

The most major thing that is missing here, for me, is a sort of 'guide level' explanation of how IME is supposed to work, and how a user of druid-shell is supposed to interact with it. Like:

each widget that can accept text input must register with the window, by calling XXXX.
each time a widget gains focus, it must set itself as active, by calling XXXX.
when the (selection, text, layout properties) of a widget change, you must notify the IME by calling XXXX.
when there is an active IME session, your widget will be given a chance to handle raw keyboard events. If the events are not handled, they will be passed to the IME, which will convert them to TextInputActions.

I think that this sort of information will be invaluable, and will be a really important artifact of this work.

Other than that, I don't really have a lot else to say, and maybe a good next step is for me to try and integrate this into druid proper, and see how that feels? I think that would give me a better general sense of how this API works for us, and whether or not there are any kinks to work out.

druid-shell/src/window.rs

cmyr · 2020-11-16T20:24:14Z

druid-shell/src/text_input.rs

+/// and `extent` to be `0`. If the user holds shift and presses the right arrow key five times, we would expect the
+/// word `hello` to be selected, the `anchor` to still be `0`, and the `extent` to now be `5`.
+#[derive(Clone, Default, PartialEq, Eq, Hash)]
+pub struct TextSelection {


This raises a pretty big question about our various impls of these different types, and whether they should be unified. I could imagine having a 'selection' type that we only use for IME, if (for instance) we wanted to support multi-selection in our main selection type, or something, but it might also be nice to simplify and try to just have a single version of these types that we use everywhere.

That would definitely be ideal! I think it will require we modify this to include last-horizontal-move-pixel-x positions, but perhaps an IME will some day want that information anyway. It does mean we'll have to more explicitly specify how pixel-x positions (is there a better name for that which you've seen) change in response to edits?

druid-shell/src/text_input.rs

druid-shell/examples/edit_text.rs

cmyr · 2020-11-24T16:21:59Z

druid-shell/examples/edit_text.rs

+    true
+}
+
+fn movement_goes_downstream(movement: TextMovement) -> bool {


maybe this makes sense as a provided method on TextMovement?

Definitely would be ideal (as would the next function) but right now the implementation is wrong; it doesn't take into account the context around the caret, which is necessary for knowing the direction of Left and Right. How would you feel about punting for a later commit, when we've figured out a little bit more of the rules for how bidi text must work?

gotcha.

I think it probably makes sense even ignoring BiDi to have a TextDirection enum in piet and to associate this with each piet TextLayout, but I'm not sure where that boundary should really be.

It does feel, though, that anyone using this code is going to need some variant of this functionality? Happy to worry about that a bit more during the integration phase, though.

druid-shell/src/platform/mac/text_input.rs

druid-shell/src/text_input.rs

cmyr · 2020-11-24T17:21:29Z

druid-shell/src/text_input.rs

+}
+
+#[allow(dead_code)]
+/// Simulates `TextInputHandler` calls on `handler` for a given keypress `event`. This circumvents the platform, and so can't


Rustdoc treats doc comments kind of like git treats commit messages; the first line is 'special', and acts as a sort of summary; for instance if you're in the docs for a module, where it lists the contents of that module, the first line is displayed beside each item.

In practice what this means is it makes sense to try and have a simple first line (like the first sentence, here) and then a newline followed by the more detailed docs.

I think this should be fixed, but if you spot a long first line I've missed somewhere, let me know!

Adds APIs for cross-platform text input, and an initial implementation of text input on macOS that allows for dead keys, IME input for Chinese, Japanese, etc. On other platforms, the text input method calls are simulated from key_down events, as a temporary measure until we can properly implement input methods on those platforms. Also adds a new druid-shell example, `edit_text`, which is a rough, LTR and single-line only implementation of the text input APIs to demonstrate that they work correctly. A massive thanks to yvt's library tcw3 (https://github.com/yvt/Stella2), which was a fantastic blueprint for the structure of this API, as well as indispensable reference when implementing the macOS text input interfaces.

Co-authored-by: Colin Rofls <[email protected]>

Also contains various small API renamings in response to code review comments.

lord · 2021-02-18T00:49:17Z

@cmyr if you're working on this soon, would it be helpful for me to rebase it?

cmyr · 2021-02-18T00:51:50Z

Only if you're feeling like you need a project; I was planning on starting a new working branch based off of this, and getting it up-to-date myself 🤷

lord · 2021-02-18T00:54:13Z

That works! I'll conserve my energy for TSF

cmyr · 2021-03-10T23:41:51Z

this has been merged as part of #1619

lord changed the title ~~Text input APIs~~ Text input API Nov 7, 2020

lord changed the title ~~Text input API~~ macOS text input Nov 14, 2020

lord force-pushed the ime_api branch 4 times, most recently from 3b9bbe0 to 392205b Compare November 14, 2020 19:51

lord marked this pull request as ready for review November 14, 2020 20:06

cmyr self-assigned this Nov 16, 2020

cmyr reviewed Nov 24, 2020

View reviewed changes

lord and others added 4 commits November 26, 2020 09:25

Update druid-shell/src/text_input.rs

cbe7ede

Co-authored-by: Colin Rofls <[email protected]>

Add a lot more documentation, switch to text module

c060fbb

Also contains various small API renamings in response to code review comments.

Log when getting an unexpected complete: command from macOS

77ca65d

lord force-pushed the ime_api branch from af3aa90 to 77ca65d Compare November 27, 2020 22:37

lord added 4 commits November 27, 2020 14:52

hopefully fix linux build

51b9b9d

extent -> active; MoveExtent -> MoveSelecting

88e5269

text input -> text field

557d363

simplifies example by using rects_for_range

6c7ff86

cmyr mentioned this pull request Dec 2, 2020

Respect platform differences when handling text input #1384

Closed

cmyr mentioned this pull request Feb 17, 2021

Copying and pasting in textbox does not work (at least on windows) #1030

Closed

cmyr mentioned this pull request Feb 23, 2021

druid-shell IME api, take two #1619

Merged

cmyr closed this Mar 10, 2021

lord deleted the ime_api branch March 10, 2021 23:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

macOS text input #1378

macOS text input #1378

lord commented Nov 7, 2020 •

edited

Loading

cmyr commented Nov 13, 2020

lord commented Nov 14, 2020

lord commented Nov 14, 2020

cmyr left a comment

cmyr Nov 16, 2020

lord Nov 24, 2020

cmyr Nov 24, 2020

lord Nov 24, 2020

cmyr Nov 25, 2020

cmyr Nov 24, 2020

lord Nov 25, 2020

lord commented Feb 18, 2021

cmyr commented Feb 18, 2021

lord commented Feb 18, 2021

cmyr commented Mar 10, 2021

macOS text input #1378

macOS text input #1378

Conversation

lord commented Nov 7, 2020 • edited Loading

cmyr commented Nov 13, 2020

lord commented Nov 14, 2020

lord commented Nov 14, 2020

cmyr left a comment

Choose a reason for hiding this comment

cmyr Nov 16, 2020

Choose a reason for hiding this comment

lord Nov 24, 2020

Choose a reason for hiding this comment

cmyr Nov 24, 2020

Choose a reason for hiding this comment

lord Nov 24, 2020

Choose a reason for hiding this comment

cmyr Nov 25, 2020

Choose a reason for hiding this comment

cmyr Nov 24, 2020

Choose a reason for hiding this comment

lord Nov 25, 2020

Choose a reason for hiding this comment

lord commented Feb 18, 2021

cmyr commented Feb 18, 2021

lord commented Feb 18, 2021

cmyr commented Mar 10, 2021

lord commented Nov 7, 2020 •

edited

Loading