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

[StoreApps/XAML] Run Game Loop and Input on a seperate Thread #5520

Merged
merged 22 commits into from
Mar 14, 2017

Conversation

nkast
Copy link
Contributor

@nkast nkast commented Feb 28, 2017

Fixes mouse, keyboard lag [UAP] touch, rotation lag & freeze (WP8.1).
#4897, #5300, #3948, #3589, #3316, #4105(?).

This probably still has a ton of bugs. It's hard to tell unless you review every line of MG codebase and run extensive tests. Unfortunetly, that's the nature of thread syncronization. I'd prefer to wait until after 3.6 (and a possible 3.6.1 service update) for this PR.

On the other hand #5352 is more predictable and solves 99% of the issues with just a couple of lines.

-Process Input on a working thread.
-Copy mouse state in a single step.
-Set mouse ButtonX1 & ButtonX2 state.
-Add method UAPGameWindow.ProcessWindowEvents()
-Add method MetroGameWindow.ProcessWindowEvents()
Key events come from the Main thread and write to Keyboard._nextKeyboardState.
Game loop thread performs a one-time copy of _nextKeyboardState to _keyboardState before Tick().
KeyboardState is a bit array of Pressed key states and there re no invalid states in the way we Set/Clear bits.
There is no need to perform more strict syncronization.
ApplicationView.GetForCurrentView() can only be called from UIThread.
@nkast
Copy link
Contributor Author

nkast commented Feb 28, 2017

@Danthekilla, @MartinZikmund, @VitezslavImrysek @jakepoz,
will you give this a run?

@jakepoz
Copy link
Contributor

jakepoz commented Feb 28, 2017

Hey nkast, will do a check here ASAP. We are also working on our own solution with @roponator so we will compare notes.

@roponator is making a fuzz tester that will run some live games overnight with very intense workloads. We want to use that we want to use as a way to verify that the multi-threading is reliable.

We are hoping to get a fix for this issue into 3.6. I feel it's a decent time to ask developers to adjust their code to Dispatch UI thread calls. Also, publishing a UWP app without this fix is just asking for lots of user complaints as we have unfortunately found out the hard way.

@Jjagg
Copy link
Contributor

Jjagg commented Feb 28, 2017

We are hoping to get a fix for this issue into 3.6.

Probably will have to be for 3.6.1 as 3.6 will be released this week.

@jakepoz
Copy link
Contributor

jakepoz commented Feb 28, 2017

@Jjagg Got it, I didn't know it was coming so soon. Either way, we are testing this fix ASAP.

@nkast Just gave this a quick test run and it seems to be working. We are going to build some of our games against this PR and run them overnight in a fuzz tester. It has been really good finding low frequency deadlocks and similar threading problems.

@tomspilman
Copy link
Member

@jakepoz - We should look at your solution as well... is it similar to this?

We will get this issue solved and shipped in a 3.6.1 release.

@jakepoz
Copy link
Contributor

jakepoz commented Mar 1, 2017

Yeah, it is similar. Instead of saving the state of each event that comes in off the main thread, and then "replaying" it like this PR does, @roponator was pausing the game loop, letting the event occur, and then resuming the game loop.

@nkast solution looks more robust, we are doing some overnight fuzz testing and will know the results shortly.

@roponator
Copy link
Contributor

roponator commented Mar 1, 2017

I think @nkast 's solution is better than mine since it doesn't lock the main loop since it stores just the last window event. I thought that may be a problem (eg if old window events would be discarded) but the ~8 hour overnight fuzz testing (all possible window change events like minimize, maximize, resize, screen rotation and then game input simulation) worked 100% fine on nkasts version so his should be used instead :)

@jakepoz
Copy link
Contributor

jakepoz commented Mar 1, 2017

Sweet, we will do a live deployment on a small game of ours and see what the Health reports look like after a week. That should make good use of the time between now and getting this into 3.6.1 :)

@jakepoz
Copy link
Contributor

jakepoz commented Mar 1, 2017

Ok, we have 2 live games updated with this PR. Will report back in a few days on the results.

@KonajuGames
Copy link
Contributor

KonajuGames commented Mar 1, 2017 via email

@VitezslavImrysek
Copy link

I am going to check tomorrow and report back the results :).

@nkast
Copy link
Contributor Author

nkast commented Mar 2, 2017

Thanks everyone for testing this.

@roponator

the ~8 hour overnight fuzz testing (all possible window change events like minimize, maximize, resize, screen rotation and then game input simulation)

That sounds interesting! +1

@jakepoz
Copy link
Contributor

jakepoz commented Mar 6, 2017

Hey,

Everything looks great on the Windows 10 desktop side. But there is a problem on Windows Phone 10 where it will crash on the first button you press:

Exception thrown: 'System.Runtime.InteropServices.COMException' in MonoGame.Framework.dll

WinRT information: Windows.Graphics.Display: GetForCurrentView must be called on a thread that is associated with a CoreWindow.

Additional information: Element not found.

That happens on the following line in InputEvents.cs

  private void PointerPressed(PointerPoint pointerPoint, UIElement target, Pointer pointer)
        {
            // To convert from DIPs (device independent pixels) to screen resolution pixels.
            var dipFactor = DisplayProperties.LogicalDpi / 96.0f;

@jakepoz
Copy link
Contributor

jakepoz commented Mar 7, 2017

@nkast Sent a Pull Request with our fix for the Windows Phone issue:

nkast#8

@nkast
Copy link
Contributor Author

nkast commented Mar 7, 2017

Thanks @jakepoz
I'll test it soon.

@nkast
Copy link
Contributor Author

nkast commented Mar 7, 2017

@jakepoz
The strange thing is that only WP10 had that issue. I tested the fix with WP8.1, W8.1, and W10 too.
Everything seems fine. In addition we removed the obsolete DisplayProperties and the division by 96.0 on each event. Thanks.

@jakepoz
Copy link
Contributor

jakepoz commented Mar 7, 2017

@nkast Kk, just confirmed the health reports and everything is good. We have this deployed to a few thousand active users without issue. Can we merge this in now?

We also have this bounty open on this issue that we can finally close out:
https://www.bountysource.com/issues/7309032-wp8-1-multitouch-slowdown

@VitezslavImrysek
Copy link

I tested this PR and the input lag seems to be fixed. I haven't noticed any problems so far.
Good job, thanks for fixing this!

@KonajuGames
Copy link
Contributor

Thanks for all the work on this. Great to have a large test userbase. I'm happy to merge this now.

@nkast
Copy link
Contributor Author

nkast commented Mar 28, 2017

@KonajuGames ,
Can we close the related issues?
#4897, #5300, #3948, #3589, #3316, #4105.

(It's required to claim the bounty on #3316)

@tomspilman
Copy link
Member

@nkast - Done!

@ddabrahim ddabrahim mentioned this pull request Jun 24, 2018
nkast added a commit to nkast/MonoGame that referenced this pull request Jun 26, 2018
…me#5520)

* Input thread

-Process Input on a working thread.
-Copy mouse state in a single step.
-Set mouse ButtonX1 & ButtonX2 state.

* -Rename _windowEvents -> _inputEvents
-Add method UAPGameWindow.ProcessWindowEvents()
-Add method MetroGameWindow.ProcessWindowEvents()

* Sync Keyboard events

Key events come from the Main thread and write to Keyboard._nextKeyboardState.
Game loop thread performs a one-time copy of _nextKeyboardState to _keyboardState before Tick().
KeyboardState is a bit array of Pressed key states and there re no invalid states in the way we Set/Clear bits.
There is no need to perform more strict syncronization.

* [UAP] Sync TextInput events

* [UAP] Get ApplicationView from GameWindow

ApplicationView.GetForCurrentView() can only be called from UIThread.

* [UAP] Dispatch SwapChainPanel.CompositionScale

* [UAP] Dispatch CoreWindow.PointerCursor

* [UAP] Update window Size before Tick()

* [UAP] Update Orientation before Tick()

* [UAP] Game loop thread

* [W8.1/WP8.1] Update window size before Tick()

* [W8.1/WP8.1] Update Orientation before Tick()

* [UAP] Update Focus before Tick()

* [W8.1/WP8.1] Dispatch CoreWindow.PointerCursor

* [W8.1/]WP8.1] Game loop thread

* [W8.1/WP8.1] Update Focus before Tick()

* Fix SwapChainBackgroundPanel size/scale bug

* Move DIP factor stuff outside of each event handler

* Move DIPs comments

* The MediaPlayer needs to get a proper dispatcher now that it is called from a different thread
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.

None yet

7 participants