Simplify & optimize style calculation #3498
Conversation
mourner
added
refactoring 🏗️
performance ⚡
mourner
force-pushed
the
simplify-calculate
branch
from
55faf5f to
54de836
Compare
|
Had to rebase to resolve a minor conflict with Flow PR, but made sure the tests pass on each commit so that they can be merged without squashing. |
| return this.interp(oldValue, value, t); | ||
| } | ||
|
|
||
| _calculateTargetValue(globalProperties, featureProperties) { |
There was a problem hiding this comment.
Why did you decide to factor this out into a separate method? What does target mean in this context?
There was a problem hiding this comment.
This code earlier lived mostly in style_declaration wrapTransitionedCalculate here. What this method does is calculate the value the property is transitioning to, as opposed to calculate which returns the current value that is transitioning from the old value to the target one with time — please see calculate above.
| ); | ||
|
|
||
| t.end(); | ||
| }); |
There was a problem hiding this comment.
Why did you delete these tests?
There was a problem hiding this comment.
I moved it into a new style_transition.test.js
A major cleanup of the style recalculation code. 50 less lines of code, much less confusion and also slightly improves performance. All commits are self-contained so review commit by commit.
One noteable architectural change is that
globalPropertiesproperties likezoomHistory,timeanddurationthat were specific to transition implementation are moved out and live where they belong. So nowglobalPropertiesare always just{zoom}, although I didn't replace it with justzoomyet because @lucaswoj anticipates other global properties in future.👀 @lucaswoj @jfirebaugh