Skip to content

Simulator performance fix: treat repeated keys as handled#405

Merged
Shchvova merged 1 commit into
coronalabs:masterfrom
ggcrunchy:HandledKeyRepeatFix
Sep 21, 2022
Merged

Simulator performance fix: treat repeated keys as handled#405
Shchvova merged 1 commit into
coronalabs:masterfrom
ggcrunchy:HandledKeyRepeatFix

Conversation

@ggcrunchy
Copy link
Copy Markdown
Contributor

In the simulator, holding the up and down keys will slow things down dramatically in Windows. For instance, throw a few emitters out and do so and they'll draw to a crawl.

This seems to concern this issue.

I don't know why only those keys seem to be affected. It did look like they might get hashed deep in their name lookup maps, but that's just a wild guess.

Anyhow, when repeating keys are detected, Solar is just backing out and trying some fallback logic. I don't know if this is necessarily expensive, but it's probably a pretty noisy event.

Since nothing else is going to handle it, it seems enough to mark this as handled and avoid those costs. (The speed increases substantially.) The Android skin / backspace logic seems to be fine, since that assumes a key up anyhow. (Incidentally, the repeat logic there is probably overkill, what with the key-is-up-and-was-up case.)

@ggcrunchy ggcrunchy changed the title Consider repeats as handled Simulator performance fix: treat repeated keys as handled Jul 20, 2022
@XeduR
Copy link
Copy Markdown
Contributor

XeduR commented Aug 5, 2022

I can confirm, after integrating these changes, the simulator no longer experiences FPS drop when holding up or down keys. This PR would resolve #358.

@scottrules44 scottrules44 requested review from scottrules44 and removed request for scottrules44 August 9, 2022 18:41
@XeduR
Copy link
Copy Markdown
Contributor

XeduR commented Sep 19, 2022

Any timeline for getting a review for this to resolve issue #358, @Shchvova & @scottrules44?

@scottrules44
Copy link
Copy Markdown
Contributor

@Shchvova I just checked out the pull, and fixes #358 in my testing. Can you look at this or merge this request?

Screen Shot 2022-09-21 at 4 58 42 PM

@Shchvova Shchvova merged commit de13c78 into coronalabs:master Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants