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

Intialization performance #21

Open
Remyoman opened this issue May 3, 2016 · 5 comments
Open

Intialization performance #21

Remyoman opened this issue May 3, 2016 · 5 comments

Comments

@Remyoman
Copy link
Contributor

Remyoman commented May 3, 2016

Currently I am using this library to add a number of checkboxes to a view controller as a visual feedback indicator towards the user. I have a view controller that contains 16 checboxes, which are set up in Storyboard with custom fill-, tint- and onCheck colors.

I noticed it took a long time to present this view controller and started profiling to look into the issue.
screen shot 2016-05-03 at 15 21 37
I then noticed that the custom attributes all make a reload call which is taking alot of time to process.
screen shot 2016-05-03 at 15 21 57

The reload call itself calls setNeedsDisplay and layoutIfNeeded, which will be done 3 times before awakeFromNib is even called in my case. What would be the most approriate solution for this problem? I assume a flag that gets set in awakeFromNib is a bit hacky, perhaps.

I can make some time tomorrow to submit a pull request with a simple fix.

@Boris-Em Boris-Em added the bug label May 3, 2016
@Boris-Em
Copy link
Owner

Boris-Em commented May 3, 2016

Hi @Remyoman,
Thanks for your feedback, this is a very good point.
Calling reload in the setters was introduced because of the issues #15 and #6.
Using a flag is definitely not very elegant. Also, awakeFromNib will not get called if the checkbox is implemented programmatically.
Essentially what we don't want to do is calling reload if the control hasn't been drawn yet. I'm not sure what the best solution really is. Maybe we could check the number of sublayers in the custom setters?

- (void)setOnCheckColor:(UIColor *)onCheckColor {
    _onCheckColor = onCheckColor;
    if (self.layer.sublayers > 0) {
        [self reload];
    }
}

This solution doesn't seem amazing either. Let me know if you can come up with something better!

@Remyoman
Copy link
Contributor Author

Remyoman commented May 4, 2016

@Boris-Em Thank you for your fast response. I agree simple if checks like these feel unelegant and I understand the need for the reload call.

The best place to make the change would be here in my opinion:

- (void)reload {
    [self.offBoxLayer removeFromSuperlayer];
    self.offBoxLayer = nil;

    [self.onBoxLayer removeFromSuperlayer];
    self.onBoxLayer = nil;

    [self.checkMarkLayer removeFromSuperlayer];
    self.checkMarkLayer = nil;

    if (self.window) {
        [self setNeedsDisplay];
        [self layoutIfNeeded];
    }
}

We only need to disable the intensive layout methods whilst the control isn't drawn yet. I will see if I can come up with something, I should have some time to look into some options today.

Edit (also reflected in code): Using a self.window check seems to work for programatically initialised checkboxes as well. It's not great, but I haven't found other ways to have a fast and clean check for this situation.

@Boris-Em
Copy link
Owner

Boris-Em commented May 4, 2016

Hi @Remyoman,
I agree with your conclusions.
Unfortunately checking for self.window before calling the layout methods introduces a new bug, where the checkbox is now redrawn if it's not currently visible.
The bug can be reproduced with the sample app by changing the animation type.

@Remyoman
Copy link
Contributor Author

Remyoman commented May 6, 2016

@Boris-Em I see, that's unfortunate.
I can't figure out anything more elegant for the moment, but I will continue looking into it when I find the time.

@Boris-Em Boris-Em added enhancement and removed bug labels May 17, 2016
@danielgindi
Copy link
Contributor

Fixed in #93

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

No branches or pull requests

3 participants