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

temporarily disable transitions for hillshade-illumination-direction #5981

Merged
merged 2 commits into from
Jan 11, 2018

Conversation

mollymerp
Copy link
Contributor

temporary workaround for #5934
cc @stevage

light-position wasn't actually transitioning, despite having transition: true in the style spec, so this won't change behavior from the current version.

when I have some spare time I will implement an interpolation function that accounts for light-position and hillshade-illumination-direction for a better solution

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page

@mollymerp mollymerp requested a review from lbud January 10, 2018 22:39
@lbud
Copy link
Contributor

lbud commented Jan 10, 2018

light-position is supposed to be interpolated using its cartesian coordinates

interpolate(a: LightPosition, b: LightPosition, t: number): LightPosition {
return {
x: interpolate.number(a.x, b.x, t),
y: interpolate.number(a.y, b.y, t),
z: interpolate.number(a.z, b.z, t),
};
}
so that degree thresholds don't cause a problem there. I would rather fix at least light properties than declare them non-transitionable 🙃 #5982

@mollymerp mollymerp changed the title temporarily disable transitions for circular properties temporarily disable transitions for hillshade-illumination-direction Jan 11, 2018
@mollymerp mollymerp merged commit 7412c0f into master Jan 11, 2018
@mollymerp mollymerp deleted the disable-light-transition branch January 11, 2018 01:16
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.

2 participants