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

Fast view snapshotting & improved UX #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mozharovsky
Copy link

@mozharovsky mozharovsky commented Jun 9, 2016

Small but useful changes

  1. iOS 7 brought a much faster way to draw the view's hierarchy into the context. Instead of rendering the layer in the context you simply call drawViewHierarchyInRect(_:, _:) method on your view which does all the magic. It's really cool! 😎

  2. Using viewWillDisappear() method isn't quite well for user experience – stopping sessions takes time, as a result we get a delay which is noticeable enough. Such things don't make users happy, so let's change the point. 😃

Thanks in advance!

@mozharovsky
Copy link
Author

mozharovsky commented Jun 9, 2016

Problem

There is one more issue I see. It's about initializing the main views (album, camera and video ones). Initialization happens in viewDidAppear() method for obvious reasons – we simply need to know the actual views' frames. But this method is called after the views are presented. This is definitely not the best way to go. View controllers are notified about the layout process through viewDidLayoutSubviews(), and it happens before the view is displayed. One thing to notice is that we have to be sure initialization is happening in the main thread (this way we won't get UI frozen).

Possible solution

I'd suggest to go for something like this:

    private var appeared = false
    public override func viewDidLayoutSubviews() {
        super.viewDidLayoutSubviews()

        if !appeared {
            albumView.frame  = CGRect(origin: CGPointZero, size: photoLibraryViewerContainer.frame.size)
            albumView.layoutIfNeeded()
            cameraView.frame = CGRect(origin: CGPointZero, size: cameraShotContainer.frame.size)
            cameraView.layoutIfNeeded()
            videoView.frame = CGRect(origin: CGPointZero, size: videoShotContainer.frame.size)
            videoView.layoutIfNeeded()

            NSOperationQueue.mainQueue().addOperationWithBlock { [weak self] in
                self?.albumView.initialize()
                self?.cameraView.initialize()
                self?.videoView.initialize()
            }
        }
    }

    override public func viewDidAppear(animated: Bool) {
        super.viewDidAppear(animated)
        appeared = true 
    }

It should work since Fusuma supports only portrait orientation (please correct me if I'm wrong).

Some thoughts

Also it's not that difficult to notice the method taking the majority of initialization time. It's initialize() on FSAlbumView. Some rough measurements will show us the following results:

     Start intiialization. time=2016-06-09T03:45:43.058 ;measurement started
     Registered cells. time=2016-06-09T03:45:43.061 ;delta=3 ms
     Checked Auth. time=2016-06-09T03:45:43.069 ;delta=8 ms
     Got images. time=2016-06-09T03:45:43.166 ;delta=97 ms
     Registered observer. time=2016-06-09T03:45:43.166 ;delta=0 ms



     Start intiialization. time=2016-06-09T03:46:01.394 ;measurement started
     Registered cells. time=2016-06-09T03:46:01.395 ;delta=1 ms
     Checked Auth. time=2016-06-09T03:46:01.396 ;delta=1 ms
     Got images. time=2016-06-09T03:46:01.418 ;delta=22 ms
     Registered observer. time=2016-06-09T03:46:01.418 ;delta=0 ms


     Start intiialization. time=2016-06-09T03:48:58.017 ;measurement started
     Registered cells. time=2016-06-09T03:48:58.028; delta=11 ms
     Checked Auth. time=2016-06-09T03:48:58.028 ;delta=0 ms
     Got images. time=2016-06-09T03:48:58.067 ;delta=39 ms
     Registered observer. time=2016-06-09T03:48:58.068 ;delta=1 ms

We can use Instruments to get more accurate picture but... There is something to do with fetching stuff. Maybe it's better to fetch some minimum count of images just for initialization purpose and then fetch others. What do you think about it?

@ytakzk
Copy link
Owner

ytakzk commented Jun 9, 2016

@mozharovsky I totally agree with you. Could you add those solutions to this pull request?

DasHutch added a commit to DasHutch/Fusuma that referenced this pull request Jun 26, 2016
Fast view snapshotting & improved UX ytakzk#49
@mamouneyya
Copy link
Contributor

Any updates on this?

@nkezhaya
Copy link
Collaborator

@mozharovsky New maintainer here. Is this PR still relevant?

@nkezhaya
Copy link
Collaborator

Not sure how to reconcile this into the current codebase, since we now have multiple callbacks. Could you update to master and fix the conflicts?

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.

4 participants