Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Clean up transition durations & time points #1888

Merged
merged 5 commits into from
Jul 14, 2015

Conversation

brunoabinader
Copy link
Member

Relevant changes:

  • Unify default transition values in MapData:
    • Added defaultFadeDuration and defaultTransitionDelay to MapData;
    • Painter & StyleParser now receives a reference to MapData;
    • As previously seen on the code:
      • 300ms is the default fade duration;
      • 0ms is the default transition duration;
    • We no longer pass the current time point to Style, since it now uses MapData.animationTime, which gets updated in MapContext::update().
    • Updated StyleParser check to use a mock MapData;
  • Pass {Duration,TimePoint} by const ref whenever possible;
  • Unify time point for Mapcontext::update:
    • Let style classes cascade use the same time point as the one used to
      recalculate style.
    • Cleaned up MapContext::update() to return early whenever possible.
    • Cleaned up MapContext::loadStyleJSON() to avoid redundant calls to style
      classes cascade.

Part of #1548.

/cc @jfirebaugh @kkaefer @tmpsantos @tmcw

void setDefaultTransitionDuration(Duration = Duration::zero());
Duration getDefaultTransitionDuration();
void setDefaultTransitionDuration(const Duration& = Duration::zero());
const Duration getDefaultTransitionDuration() const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning a const integral type isn't meaningful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I guess that went through on my regular expression 👻 will remove that.

@tmpsantos
Copy link
Contributor

👍

- Let style classes cascade use the same time point as the one used to
  recalculate style.
- Cleaned up MapContext::update to return early whenever possible.
- Cleaned up MapContext::loadStyleJSON to avoid redundant calls to style
  classes cascade.
This list stores different values for the same property, but the
property itself remains the same.
@brunoabinader brunoabinader force-pushed the transition-unify-refactoring branch 2 times, most recently from 2327d46 to a34e6cd Compare July 14, 2015 13:47
@brunoabinader brunoabinader force-pushed the transition-unify-refactoring branch 2 times, most recently from 2bda4f0 to 9ad8c41 Compare July 14, 2015 13:50
Relevant changes:
 - Added 'defaultFadeDuration' and 'defaultTransitionDelay' to MapData;
 - Painter & StyleParser now receives a reference to MapData;
 - As previously seen on the code: 300ms is the default fade duration
   and 0ms is the default transition duration;
 - We no longer pass the current time point to Style, since it now uses
   MapData.animationTime, which gets updated in MapContext::update().
 - Updated StyleParser check to use a mock MapData;
@brunoabinader brunoabinader merged commit 0ea8a37 into master Jul 14, 2015
@mourner mourner removed the ready label Jul 14, 2015
@brunoabinader brunoabinader deleted the transition-unify-refactoring branch July 14, 2015 14:18
brunoabinader added a commit that referenced this pull request Jul 14, 2015
Added a setter/getter for default transition delay, in the same fashion
as the default transition duration.

Spin-off from #1888. Fixes #302.
brunoabinader added a commit that referenced this pull request Jul 21, 2015
Added a setter/getter for default transition delay, in the same fashion
as the default transition duration.

Spin-off from #1888. Fixes #302.
AndwareSsj pushed a commit to AndwareSsj/mapbox-gl-native that referenced this pull request Nov 6, 2015
Added a setter/getter for default transition delay, in the same fashion
as the default transition duration.

Spin-off from mapbox#1888. Fixes mapbox#302.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants