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

Replaced main operation queue with user initiated queue #19

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

fajdof
Copy link
Contributor

@fajdof fajdof commented Mar 12, 2018

Hi @scihant ,
Performing motionManager.startDeviceMotionUpdates on main queue caused very strong lagging on iPhone X. I updated it so it uses a user initiated queue which I think is suitable for this case. From Apple's documentation:
"Used for performing work that has been explicitly requested by the user, and for which results must be immediately presented in order to allow for further user interaction."

@fajdof fajdof closed this Mar 12, 2018
@fajdof fajdof reopened this Mar 12, 2018
@scihant
Copy link
Owner

scihant commented Mar 13, 2018

Hi,

I've just tested the code on an iPhoneX before and after your changes, and haven't noticed any difference. In both cases it seems to work properly. Are you sure that the library is the cause of the delay and you're not doing something in background that lowers the performance?

@fajdof
Copy link
Contributor Author

fajdof commented Mar 13, 2018

Hi,
I just have a view controller with CTPanoramaView and a loaded image. When I move the iPhone X (iOS 11.2.5) the image moves some 1-3 seconds later. Tested on iPhone 6 and 6s and it seems to be working fine there. When I replace the main queue with user-initiated, everything works smoothly (didn't change anything else).

Also noticed that when running with main queue if I increase the deviceMotionUpdateInterval, the lagging becomes shorter. But switching to different queue completely removes lagging.

@scihant
Copy link
Owner

scihant commented Mar 13, 2018

Ok, it wouldn't hurt anyway.

In your code, whenever the control method is changed, a new queue is created. I think it would be better if you make that queue a lazy member just like cameraNode is and then use it directly when starting motion updates.

    private lazy var opQueue: OperationQueue = {
        return ...
    }()

If you make that change and amend your commit (so that the result is a single commit again), I'll merge it.

Thanks.

@fajdof
Copy link
Contributor Author

fajdof commented Mar 13, 2018

Hi,
Sure, it's better if it's lazy. Updated the pull request with amended commit.

@scihant scihant merged commit 797bb18 into scihant:master Mar 13, 2018
@scihant
Copy link
Owner

scihant commented Mar 13, 2018

Thank you for your contribution.

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.

2 participants