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

iOS7 Add pointer device support #4032

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

larsamannen
Copy link

@larsamannen larsamannen commented Jun 21, 2022

No description provided.

larsamannen added 4 commits Jun 21, 2022
Add the GameController framework to the project and enable support for
controller user interaction in the Info.plist file. This allows for
Game Controller compatible devices to notify the application when
connected.
Setting this property to true indicates the view controller’s preference
to lock the pointer, although the system may not honor the request.

For the system to consider locking the pointer:
The scene must be full screen, not in Split View or Slide Over, with no
other apps in Slide Over.
The scene must be in the UISceneActivationStateForegroundActive state.

The ScummVM iOS7 client fulfills the above so the pointer is locked.
Locking the pointer hides the OS cursor (the dot), however that's wanted
since the ScummVM engine draws its own pointer.
Let the view keep the current pointer position as a property, allowing
it to be modified by controllers.
sev-
sev- approved these changes Jun 21, 2022
Copy link
Member

@sev- sev- left a comment

Good code quality, only one minor note. I would also like as part of this PR to modify the port documentation at doc/docportal/other_platforms/ios.rst

@sev- sev- requested a review from criezy Jun 21, 2022
@ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Jun 21, 2022

For game controllers, I'd recommend sending controller events to allow the iOS port to make better use of the keymapper instead of using a hard-coded mapping in the iOS 7 backend. There are some details about this here: https://wiki.scummvm.org/index.php?title=Keymapper

larsamannen added 9 commits Jun 23, 2022
Add a GameController base class which handles user inputs from a
controller. The input is either a pointer move or a button action.
If the input is a pointer move, make sure that the move is within
valid coordinates in the game (respecting the resolution which is
most probably lower than the view resolution).
Move touch inputs to a TouchController class to move some logic from the
iPhoneView class. Only do this for touches on screen since connected
trackpads can generate touches as well. The latter ones are of type
UITouchTypeIndirectPointer while touches on screen are of type
UITouchTypeDirect. They are separated thanks to the preference key
UIApplicationSupportsIndirectInputEvents set to YES in Info.plist.
Without the preference above, there is no way to distinguish touches
from screen from a trackpad.
Add support for mouses using the GameController framework. This requires
iOS 14 and up. The trackpad on the magic keyboard to iPads is connected
as a mouse and of course other connected mouses.

The mouse movements triggers calls to the mouseMovedHandler code
block. The calls delivers delta movements on the X and Y axis from the
last pointer position. It doesn't keep track on where the pointer is in
the view. That's where pointerPosition property in the iPhoneView comes
into place.
Joystick actions are suitable for joysticks and gamepads where the
movements are updated by a controller stick. On gamepads that's usually
a thumbstick.

Add joystick events which can be triggered by each implemented
controller that should utilize the ScummVM Joystick events.
Add support for Extended Gamepad controllers. What defines extended
gamepad controllers can be found here:
https://developer.apple.com/documentation/gamecontroller/gcextendedgamepad

Support has been added for controlling the pointer position using the
left thumbstick, left clicks using the A-button and right clicks using
the B-button. Also the Main menu can be accessed using the Home/Menu
button.

The thumbstick values are received when changed, however if holding the
thumbstick in the same position the valueChangedHandler will not be
called. Therefore store the X- and Y-axis values and begin to poll
readings of the stored values for as long as the thumbstick is out of
the center position.
Trigger EVENT_INPUT_CHANGED when devices connects to make the ScummVM
engine update the hardware input set.
@larsamannen larsamannen force-pushed the ls_ios7_add_pointer_device_support branch from d13f9e6 to 1f9e82e Compare Jun 23, 2022
@larsamannen
Copy link
Author

@larsamannen larsamannen commented Jun 23, 2022

By the way, I forgot to mention that I’ve tested the implementation on an iPad Pro, using the magic keyboard, a Magic Mouse and a Nimbus controller.

Copy link
Member

@criezy criezy left a comment

I had a very quick look and will look more in details in the next few days.
Overall the code quality looks good.

I already added a few comments though, and it also looks like you intended to squash 94d0e74 but forgot?

Note that I will not be able to test the code as my iPad is too old and stuck to iOS 12. But I can at least check that it compiles and still works.

@@ -422,6 +423,9 @@ - (id)initWithFrame:(struct CGRect)frame {

[self setupGestureRecognizers];

if (@available(iOS 14.0, *)) {
Copy link
Member

@criezy criezy Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past we had issues using this as old compilers do not have the @available() keyword. So we had to protect this with #if __has_builtin(__builtin_available) as well.

However I don't know if that is still relevant or if in practice nowadays we can assume that @available() is available.

Copy link
Author

@larsamannen larsamannen Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try to find any information about it. Xcode adds these checks automatically.

Copy link
Member

@criezy criezy Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to indicate here that after my compilation tests from today, this was not an issue for me. So I am guessing this would only be an issue for very old Xcode versions.

Copy link
Member

@criezy criezy Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was introduced in llvm 5.0: https://clang.llvm.org/docs/LanguageExtensions.html#objective-c-available, which was released in September 2017.

Copy link
Author

@larsamannen larsamannen Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How old Xcode versions / LLVM versions do we want to support? The cost and work to support it will grow and it will be hard to keep backwards compatibility.

Copy link
Member

@criezy criezy Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a history of supporting old platforms. However that happens when we have a dedicated developer spending time to keep ScummVM working on those. And even then they might require a modern compiler (we at least require support for c++11).

For iOS we don't even have a maintainer for modern iOS and Xcode versions, let alone old ones. So I don't have an issue with dropping support for using very old Xcode version that do not support the @available keyword (I suspect that would mean any version older than Xcode 9, which is probably the first version to support it since it was released in 2017).

@@ -555,6 +556,7 @@ void XcodeProvider::setupFrameworksBuildPhase(const BuildSetup &setup) {
frameworks_iOS.push_back("CoreGraphics.framework");
frameworks_iOS.push_back("CoreFoundation.framework");
frameworks_iOS.push_back("Foundation.framework");
frameworks_iOS.push_back("GameController.framework");
Copy link
Member

@criezy criezy Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configure and ports.mk also need to be updated for those not using Xcode to build the package (for example on our buildbot).

Copy link
Author

@larsamannen larsamannen Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add that

@criezy
Copy link
Member

@criezy criezy commented Jun 27, 2022

I am getting the following compilation errors with my version of Xcode (Xcode 9.2 / SDK for iPhoneOS 11.2):

scummvm/backends/platform/ios7/ios7_gamepad_controller.mm:93:31: Property 'leftThumbstickButton' not found on object of type 'GCExtendedGamepad *'; did you mean 'leftThumbstick'?
scummvm/backends/platform/ios7/ios7_gamepad_controller.mm:93:74: Assigning to 'GCControllerDirectionPadValueChangedHandler _Nullable' (aka 'void (^)(GCControllerDirectionPad * _Nonnull, float, float)') from incompatible type 'void (^)(GCControllerButtonInput * _Nonnull, float, BOOL)'
scummvm/backends/platform/ios7/ios7_gamepad_controller.mm:97:31: Property 'rightThumbstickButton' not found on object of type 'GCExtendedGamepad *'; did you mean 'rightThumbstick'?
scummvm/backends/platform/ios7/ios7_gamepad_controller.mm:97:75: Assigning to 'GCControllerDirectionPadValueChangedHandler _Nullable' (aka 'void (^)(GCControllerDirectionPad * _Nonnull, float, float)') from incompatible type 'void (^)(GCControllerButtonInput * _Nonnull, float, BOOL)'
scummvm/backends/platform/ios7/ios7_gamepad_controller.mm:109:31: Property 'buttonMenu' not found on object of type 'GCExtendedGamepad *'
scummvm/backends/platform/ios7/ios7_mouse_controller.mm:30:2: Unknown type name 'GCMouse'
scummvm/backends/platform/ios7/ios7_mouse_controller.mm:53:40: C++ requires a type specifier for all declarations

Looking at the documentation it appears that

  • leftThumbstickButton and rightThumbstickButton were introduced in iOS 12.1
  • buttonMenu was introduced in iOS 13.0
  • GCMouse was introduced in iOS 14.0

Once compiled (after commenting out the code causing the errors above) it works mostly well, but there seems to be a regression in direct touch mode (when touchpad mode is disabled). With current master code the cursor immediately moves to where we touch on the screen. With this pull request it doesn't immediately move to where we touch, and only move there if we touch and then move a bit the finger while still touching. I find this a bit confusing and made it harder to press buttons in the menu for example.

@larsamannen
Copy link
Author

@larsamannen larsamannen commented Jun 27, 2022

Hmmmm, the gamepad controller support should not be compiled on iOS versions older than 14 due to the API-check in the header file. But I will look at it.

I will also check the regression problem with touchpad mode disabled as well.

I’m currently on a trip so I will look into it late next week.
Thanks for the review

@sev-
Copy link
Member

@sev- sev- commented Jul 2, 2022

@larsamannen hope you will be able to look into it soon

@larsamannen
Copy link
Author

@larsamannen larsamannen commented Jul 2, 2022

@larsamannen hope you will be able to look into it soon

I’m currently on a vacation trip. Will be able to look into it end of next week if that’s ok?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants