-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Ensure images always fade in completely, fix broken "raster-fade-duration" property #3532
Conversation
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.
A ton of raster render tests are now failing. Also, Style
sets rasterFadeDuration
in _recalculate
— could there be a better place for the constant?
@@ -156,6 +156,8 @@ class ImageSource extends Evented { | |||
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.LINEAR); | |||
gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.LINEAR); | |||
gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, gl.RGBA, gl.UNSIGNED_BYTE, image); | |||
this.map.animationLoop.set(this.tile.timeAdded + this.map.style.rasterFadeDuration - Date.now()); |
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.
Why did animation loop work for raster fades before adding this line? Do we set the loop in several places for this?
4647c8c
to
da7add6
Compare
Because of the # of failing tests and the additional workload of safely deprecating this feature, I have decided to change direction and fix our support for |
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.
Looks so much cleaner now! Merge on green.
As explained in #3526 , Mapbox GL JS does not properly implement
raster-fade-duration
. This PR fully removes support for "raster-fade-duration" and thereby ensures that images always fade in completely.cc @jfirebaugh @mollymerp @mourner @mapsam
Launch Checklist
write tests for all new functionalitywe do not have any infrastructure to test animations