This repository has been archived by the owner on Aug 8, 2023. It is now read-only.
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refs #5983: gate all viewport changes with equality check first
- Loading branch information
4ef336e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re: all your commit comments here to date @1ec5, per 4ef336e#commitcomment-18678302 I am trying more to introduce a pattern of checking the viewport using utility methods rather than doing all sorts of potentially lengthy direct comparisons right in each setter.
4ef336e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that approach is untenable for the time being, given #2259. Furthermore, this approach merely adds more boilerplate code and points of failure for any given check. If we only discovered #2259 once this branch lands, we’d have a wild goose chase involving power consumption before understanding the root cause.
4ef336e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, there’s no need for lengthy direct comparisons: in
-setDirection:
, you only need to check whether the new direction equals the old one – no need to look at the rest of the state. This is a best practice for writing a setter, anyways. If you want to be sure that callingeaseTo()
with the same camera options results in no animation, that would be an ideal candidate for an mbgl unit test.