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

Hit test for a single tap now checked on main thread #1209

Merged

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented Mar 28, 2019

Fixes #1207

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

Test cases for the SDLTouchManager were updated.

Summary

Checking if a single tap is inside a UIView no longer causes the app to freeze for a few seconds. All checking is now done on the main thread.

Changelog

Breaking Changes
  • n/a
Bug Fixes
  • Checking if a single tap is inside a UIView no longer causes the app to freeze for a few seconds. All checking is now done on the main thread.

CLA

@NicoleYarroch NicoleYarroch added the bug A defect in the library label Mar 28, 2019
@NicoleYarroch NicoleYarroch self-assigned this Mar 28, 2019
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

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

Changing the thread a public API is called on can have an effect on the way the API is used (imagine if a consumer of the touch manager callback used dispatch_sync() to the main queue, this change would start crashing their app). Therefore, I believe it may be a major API change. Would changing the call site in the haptic manager to dispatch to the main queue be a better fix here?

@NicoleYarroch NicoleYarroch added this to the 6.2.1 milestone Apr 2, 2019
@NicoleYarroch NicoleYarroch changed the title Bugfix/issue 1207 haptic input checked on wrong thread Hit test for a single tap now checked on main thread Apr 2, 2019
@NicoleYarroch
Copy link
Contributor Author

After doing more testing, I realized that only the single tap touch was not being hit tested on the main thread. I added a HAX to perform the hit test on the main thread and then notify the delegate on the background thread.

SmartDeviceLink/SDLTouchManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLTouchManager.m Show resolved Hide resolved
SmartDeviceLink/SDLTouchManager.m Show resolved Hide resolved
@joeljfischer joeljfischer merged commit e222a5b into develop Apr 3, 2019
@joeljfischer joeljfischer deleted the bugfix/issue_1207_haptic_input_checked_on_wrong_thread branch April 3, 2019 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants