Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DolphinQt: Reduce latency of TAS input's controller input passthrough #10113

Merged
merged 1 commit into from Sep 20, 2021

Conversation

JosJuice
Copy link
Member

Fixes https://bugs.dolphin-emu.org/issues/12676.

Needs testing to see if this impacts performance when controller inputs are changing.

Fixes https://bugs.dolphin-emu.org/issues/12676.

Needs testing to see if this impacts performance when controller
inputs are changing.
@leoetlino
Copy link
Member

leoetlino commented Sep 19, 2021

How feasible would it be to wrap the entire series of GetButton calls in e.g. GCTASInputWindow::GetValues inside QueueOnObject instead of queuing every UI control update one by one? Seems less expensive than constructing so many dummy QObject and connections and locking/unlocking dozens of times.

Considering GCTASInputWindow::GetValues and WiiTASInputWindow::GetValues already call Qt functions — which is probably not thread safe — maybe we should just wrap the call to GetValues with QueueOnObject?

@JosJuice
Copy link
Member Author

JosJuice commented Sep 19, 2021

I would prefer to avoid making large changes to this system until PR #9624 has been merged (or closed without being merged), since that PR changes the surrounding code quite a bit.

Regarding the performance, doing it the way we're currently doing it does mean that there is zero cost on frames where none of the inputs have changed, but a bigger cost the greater the number of changed inputs is. I'm not sure if we want to pay the cost of one blocking cross-thread call even on frames where no inputs are changed. I especially wouldn't want us to pay the cost before calling isVisible, since that would force us to pay the cost regardless of whether the user actually is using TAS input. But you are right in that calling getters on Qt widgets isn't actually thread-safe...

@leoetlino
Copy link
Member

Hm, I guess this is probably fine as a temporary solution...

The fact we are calling Qt functions (and not just getters either) from the wrong thread is still highly problematic, but at first glance #9624 seems to fix that.

@leoetlino leoetlino merged commit 28e6e87 into dolphin-emu:master Sep 20, 2021
10 checks passed
@JosJuice JosJuice deleted the tas-input-latency branch September 20, 2021 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants