search

flowkey / UIKit-cross-platform

Cross-platform Swift implementation of UIKit, mostly for Android
MIT License
598 stars 40 forks source link

Improve Touch (Part 2 - Multitouch) #283

Open rikner opened 5 years ago

rikner commented 5 years ago

Motivation (current vs expected behavior)

Adds support for multitouch

Currently the branch causes huge slowdowns with multitouch, we may need to reorganise some code to make this efficient

ToDo

Please check if the PR fulfills these requirements

ephemer commented 5 years ago

@rikner what is the status of this PR? I'm marked to review it but it's missing a description etc. Can you please update it when you get a chance and mark me again for review if it's ready? I'm happy to do some work on this at some point myself too if needed

rikner commented 5 years ago

@ephemer In the initial PR (#265) we refactored the timestamps and introduced multitouch, but had performance issues. I split the initial PR into #282 and this one. Currently the performance issues still exist when using multiple fingers. That's the most important issue we should solve.

codecov[bot] commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@5bb40a3). Click here to learn what that means. The diff coverage is 27.35%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #283   +/-   ##
=========================================
  Coverage          ?   51.17%           
=========================================
  Files             ?       87           
  Lines             ?     3224           
  Branches          ?        0           
=========================================
  Hits              ?     1650           
  Misses            ?     1574           
  Partials          ?        0
Impacted Files Coverage Δ
Sources/UIScreen.swift 9.09% <ø> (ø)
Sources/UIView+animate.swift 78.37% <ø> (ø)
Sources/UIApplicationDelegate.swift 0% <ø> (ø)
Sources/UINavigationBarAndroid.swift 57.14% <ø> (ø)
Sources/AVPlayerItem+Mac.swift 0% <ø> (ø)
Sources/UIWindow.swift 71.79% <ø> (ø)
Sources/UIAlertAction.swift 0% <ø> (ø)
Sources/SDL2-Shims.swift 0% <ø> (ø)
Sources/DisplayLink.swift 0% <0%> (ø)
Sources/UIViewAnimationGroup.swift 63.63% <0%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5bb40a3...a7719cb. Read the comment docs.

rikner commented 4 years ago

@ephemer @janek and me merged the latest master and ran this on a device to reproduce the performance issue. As far as I remember the player got unresponsive for a while when using multiple fingers, but that's not the case anymore. We also looked at the Profiler in Android Studio and saw that the CPU usage was not different from when using single vs multi-touch. Could it be that this has been solved by some other PR?

ephemer commented 4 years ago

Hey @rikner thanks for the update here! I would like to try it especially on a device with weak performance to see, it should be immediately obvious whether there are still issues here.

AFAIK we haven't changed anything in another PR so I'd be kind of surprised if the issues have spontaneously gone away. Also curious as to whether you tried this with our app or with the test app (which might hint whether the performance issues are affected by a larger / deeper view hierarchy). Feel free to ping me when you have time and we can have a look at this together

rikner commented 4 years ago

@ephemer @janek and me also tested it on the rather crappy samsung phone we have in the office. we did not see anything suspicious in the CPU usage. But let's compare the CPU usage between this branch and master again. I'll get to you with that.