kotcrab / vis-ui

libGDX UI toolkit
Apache License 2.0
716 stars 128 forks source link

[VisUI] Input cursor blinking with non continuous rendering #282

Open metaphore opened 6 years ago

metaphore commented 6 years ago

I recently started to develop new skin for desktop app optimized for non continuous rendering behavior and noticed that VisTextField's input cursor is not blinking. I shortly found the code under VisTextField#blink() that omits timer logic for that case.

It was actually a quite obvious solution to me and I would go the same way, but this was before I really get in touch with the widget and eventually find out it is little hard to spot the cursor position when it isn't animated at all. I had have an idea to offer to implement some sort of a middle case, when user decides if cursor should blink during non continuous rendering state. But few moments later I thought about how rare this blink really happens and also it only appears when field is selected. So maybe would be better to drop those lines

if (!Gdx.graphics.isContinuousRendering()) {
    cursorOn = true;
    return;
}

and just call Gdx.graphics.requestRendering() when the blink triggers?

What do you think?

kotcrab commented 6 years ago

This bit comes from libGDX and I haven't really used non continuous rendering. You should probably also report it in libgdx repo. That said I think cursor blinking is essential even when using non continuous rendering to save resources. It should just blink normally in that mode. Do you want to make PR for this change?

metaphore commented 6 years ago

Oh, you're right, I didn't think it could have come from the original LibGDX code. I made PR and opened issue for the LibGDX repo and only than suddenly realized that blink() calls from the render() and to solve this we should go by other approach.

I see Timer as a best candidate to solve this, but no sure if it's safe enough to use with Scene2d. I saw that you use Timer for ToastManager and that should generally works fine, just require a little more checks due to Timer callbacks are not bound to actor lifecycle and may be called after host is already removed from the stage.

kotcrab commented 6 years ago

libgdx is using Timer.schedule in TextField for repeating keys, in TooltipManager (which was added after VisUI tooltips so it's different implementation) and in DragScrollListener. I would say it's pretty safe to use in scene2d.

metaphore commented 6 years ago

@kotcrab please have a look at https://github.com/libgdx/libgdx/pull/5027 If that's fine with you, I can prepare same PR for VisUI

metaphore commented 6 years ago

I think we can skip this till the next LibGDX release, because Nate modified Timer and made this properly.

kotcrab commented 6 years ago

Alright, if you think this is not required right now we can wait for next libGDX release.