azul icon indicating copy to clipboard operation
azul copied to clipboard

Added basic cursor to TextInput

Open mjhouse opened this issue 5 years ago • 3 comments

I removed a lot of the cruft from the previous iteration of the cursor and tried to limit the changes a little bit. The LayoutResult is being moved into the WindowState and exposed to callbacks. This happens at the bottom of update_display_list, right before it would have gone out of scope- keeping it around shouldn't change performance too much.

Issues:

  1. Cursor height isn't currently set. It's always the same height as the input widget, because I would prefer to set it programmatically but allow users to override it in CSS, and that isn't possible right now as far as I know.

  2. Cursor is a little bit off when text overflows due to #142.

  3. The cursor doesn't blink. With CSS this would have required animation/keyframes, and doing it programmatically would have been involved and hacky.

  4. The cursor is always one-character delayed from the real position. This is because:

    1. The glyphs are laid out on the screen and LayoutResult is passed to the WindowState.
    2. The callbacks are triggered, and they calculate the position of the cursor.
    3. The callbacks change the text/position.
    4. -> back to i

    So when the text is displayed, the cursor is always positioned as it would have been in the last render. This is an issue that I'm not sure how to fix. My last attempt at this was re-calculating text width in the TextInputState in order to position the cursor for the next render, but it would be better not to re-do the text layout again if we don't have to.

mjhouse avatar Apr 10 '19 16:04 mjhouse

Thanks for reworking it, this looks a lot better than the previous attempt, good job. I'll take a look later for minor changes, but overall it looks decent. The "one frame delay" problem - yeah, it's a problem and I'm not sure how to solve it (maybe using inline layout later on instead of absolute / relative positioning, so that the position of the cursor is treated like a "span" element), but it's certainly better than nothing.

I wouldn't necessarily put the LayoutResult it inside the WindowState, I'd just reference it as a new field to the CallbackInfo (like struct CallbackInfo { layout: &'a LayoutResult }) - I essentially want the CallbackInfo to be more or less like the "inspect element" tool, so that you can query the computed layout (the position / size of on-screen rectangles) and computed CSS properties. I'd avoid mutating / setting the window state if possible, because (a) the window state should only describe the actual window, i.e the position / size of the actual window and (b) mutating things leads to bugs very quickly, ex. when in a future revision someone accidentally forgets to update the window state it can lead to update bugs.

But for a first attempt it's not too bad.

fschutt avatar Apr 10 '19 17:04 fschutt

Thanks, I'll go look at the CallbackInfo and find a logical place to add the LayoutResult to it.

Edit: a logical place in the code to snag the LayoutResult, I mean.

mjhouse avatar Apr 10 '19 18:04 mjhouse

I moved the LayoutResult to UiState so that it gets passed to callbacks in CallbackInfo. Since the render happens after the callbacks, I had to put it on some existing struct so that it would be available in the next pass.

mjhouse avatar Apr 11 '19 10:04 mjhouse