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

[WIP] Add example: animate a line #3788

Closed
wants to merge 61 commits into from
Closed

Conversation

YunjieLi
Copy link
Contributor

Per #1513 I created a new example of animating a line.

I rewrote the Mpabox.js example using GL JS and requestAnimationFrame so it's smoother. I also add the play and stop buttons which is not in the point animation example.

Here is a demo.

Still working on:
[ ] Slight flickering when I stop and resume animation
[ ] Bug in leaving the tab inactive and come back
[ ] Pan the map to follow animation: speed control?

cc @geografa @mollymerp

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @YunjieLi! In addition to the other comments, the sin line continues onto other copies of the world outside of the viewport and I'm afraid this might confuse people. I think it would be preferable to choose a route / path that stays within the viewport

@@ -0,0 +1,128 @@
--- layout: example category: example title: Animate a line description: Animate a line by updating a GeoJSON source on each frame. tags: - layers - sources ---
Copy link
Contributor

Choose a reason for hiding this comment

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

this section needs to be formatted like the other examples (not all on one line, etc)

<button id='play' class='pause'></button>

<script>
mapboxgl.accessToken = 'pk.eyJ1IjoieXVuamllbGkiLCJhIjoiY2lxdmV5MG5rMDAxNmZta3FlNGhyMmpicSJ9.CTEQgAyZGROcpJouZuzJyA';
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need an access token here. a token is automatically added by jekyll when it compiles the examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I forgot to remove it from the gist

'line-color': '#3cc',
'line-width': 5,
'line-opacity': .4
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hard to see on the map -- I would make it a brighter color or more opaque and/or change the basemap.

// map.panBy([60, 0]);

// Request the next frame of the animation.
animation = requestAnimationFrame(animateLine);
Copy link
Contributor

Choose a reason for hiding this comment

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

There's some interesting discussion about using using requestAnimationFrame with GeoJSONSource#setData in #3692

startTime = performance.now() - progress;
animation = requestAnimationFrame(animateLine);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be open to removing the "play" button in order to reduce the # of lines of code in this example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it according to @geografa's comment that user wants more control over the animation. But I do think I can simplify into just a 'play/replay' button + no looping for animations. Do you think that would make it clean enough?

var x = progress / speedFactor;
// draw a sine wave with some math.
var y = Math.sin(x * Math.PI / 90) * 40;
// append new coordinates to the lineStrin
Copy link
Contributor

Choose a reason for hiding this comment

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

You're missing a "g" on "line string"

};
}

progress = time - startTime;
Copy link

Choose a reason for hiding this comment

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

This is the source of the inactive tab bug-- when on another tab, requestAnimationFrame does not get called, so your progress variable makes a big jump when you return and you don't add any points in between. Consider something like this to limit the maximum timestep between frames:

var startTime;
var lastTime;
var progress = 0;

function animateLine(time) {
  var dT = time - lastTime;
  if(dT > 30) dT = 30; // Don't jump too far between animation frames, even if the browser tab is inactive
  progress += dT;
  lastTime = time;

  // The rest of what you have should work the same

}

Copy link
Contributor

Choose a reason for hiding this comment

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

You can also do this, which should work, though I haven't thoroughly tested it - which would reset the timer when a tab changes visibility:

document.addEventListener("visibilitychange", function() {
  startTime = performance.now();
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @kronick @tmcw! I'll research into both suggestions a little more. I think visibilitychange sounds more straightforward but am not sure if there are other things that can interrupt requestAnimationFrame besides inactive tabs.


// requestAnimationFrame pauses when the tab is inactive
// reset startTime and progress when switched back
document.addEventListener('visibilitychange', function(){
Copy link
Contributor

Choose a reason for hiding this comment

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

this call shouldn't be inside of animateLine: animateLine will be called many times but document.addEventListener should be called once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @tmcw ! I just pushed some new edits if you want to take a look again?

@YunjieLi
Copy link
Contributor Author

@mollymerp which branch should I rebase this to?

@mollymerp
Copy link
Contributor

@YunjieLi you can rebase against mb-pages 👍

@YunjieLi
Copy link
Contributor Author

YunjieLi commented Jan 26, 2017

Also - currently the map shows a straight line between the starting point and x,y by the time the window is loaded, versus starting animation until the map is ready(?). Will look into it next.
image

@1ec5 1ec5 added GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform mapbox.js → GL JS For feature parity with mapbox.js or Leaflet labels Feb 15, 2017
pirxpilot and others added 12 commits May 4, 2017 18:41
commit 5ceec40 did not update yarn.lock after changing package.json
as a result every time dependencies are install yarn changes yarn.lock
now

ideally package.json dependencies should be changed using `yarn add`,
`yarn remove` or `yarn upgrade` to keep package.json and yarn.lock in
sync
No error or warning is printed for missing patterns.

fix #4660
We switched to 16-bit coordinates in attributes in a84083b, so there's no longer any reason to require that they are divisible by four.

The integration test updates here are due to:

* Underspecified behavior for overlarge text-halo-width values which was perturbed by this change
* A bug in icon-text-fit logic where the width after extending mod 4 was used, instead of the intrinsic icon width
* Other random and insignificant perturbations
This makes pattern usage more like icons, and will be necessary for data-driven *-pattern properties.
Manually bind a_data at position 0 for symbolSDF shader. Binding unused/undefined attributes to position 0 causes symbols to not render in Safari.
ChrisLoer and others added 23 commits May 25, 2017 16:01
…rom camera to the center of the tile. Recalculating the shape allows the collision approximations in the shaders to remain more accurate.
…rid.length` will always be undefined, but if `grid.keys.length === 0` then we can skip running through the query geometry.
When a label is twice as far away, make it 75% of its original size, instead of 50% of its original size.
Perspective effects may make line labels show at an "effective scale" of less than 1 even though the tile isn't underzoomed. Collision boxes are only generated once at tile parsing time, so they need to account for the possibility of being highly pitched even if the map is not currently pitched.
 - Reduces amount of background CPU usage during rotation/pitch operations at cost of somewhat increased latency in collision detection
 - Adds symbol re-placement during pan operations to support label size changing with distance
 - Symbol re-placement for "tile distance" changes is restricted to pitches > 25 degrees
…hat won't render well. Hardwire to value of 1.5 for viewport-pitch-aligned line labels. Depending on the size of the viewport, this means something like "start hiding line labels at the top of the screen as you approach a pitch of 50 degrees".
…ne collision features if the map isn't pitched.
- Necessary because pitched/wrapped tiles now have different CollisionTiles
- Introduces potential for more flicker while crossing dateline at zoom level 0
- To preserve the ImageSource/VideoSource wrapping behavior, we can now bind the same image/video to multiple (wrapped) tiles
- query_features#mergeRenderedFeatureLayers adds a fair amount of extra work to preserve existing query behavior with wrapped tiles: i.e. don't return a feature twice if it's returned from both tiles.
- Flip the tile order of results in box-cutting-antimeridian-z0. This was previously based on the order provided by SourceCache#getIDs, which was only strictly defined for zoom level. It's now defined by query_features#sortTilesIn
This eliminates precision-related disagreement between collision box colors and whether symbols actually show.
…inzoom, etc.

This resolves an iOS rendering issue where right at the edge of a collision some vertices could be hidden while others showed. It's possible the underlying issue could still be exposed when the un-rounded perspective_zoom_adjust value is almost exactly at a multiple of .1, but if so it will be much less common.
* fix update edit link, add query parameters for map feedback

* update url structure

* correct url in test

* add access_token param

* update tests w access_token param

* add pitch and bearing

* fix hash, make consistent w existing hash option

* use rounding instead of toFixed to get rid of .00s, update tests

* make hash conform to feedback expectations

* change hash back to private property
YunjieLi pushed a commit that referenced this pull request Jun 2, 2017
@YunjieLi YunjieLi mentioned this pull request Jun 2, 2017
@YunjieLi
Copy link
Contributor Author

YunjieLi commented Jun 2, 2017

Somehow I couldn't get this running locally anymore, so moving to #4780!

@YunjieLi YunjieLi closed this Jun 2, 2017
@YunjieLi YunjieLi deleted the example-line-animation branch June 2, 2017 21:59
mollymerp pushed a commit that referenced this pull request Jun 7, 2017
* copy and paste from pr #3788

* gentle tweaks

* minor edits to example

* reverse yarn.lock

* styling update

* rename pauseAnimation

* rename to resetTime

* fix comments

* copy edits to comments
lbud pushed a commit that referenced this pull request Jun 9, 2017
* copy and paste from pr #3788

* gentle tweaks

* minor edits to example

* reverse yarn.lock

* styling update

* rename pauseAnimation

* rename to resetTime

* fix comments

* copy edits to comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GL native → GL JS For feature parity with Mapbox Maps SDK on a native platform mapbox.js → GL JS For feature parity with mapbox.js or Leaflet
Projects
None yet
Development

Successfully merging this pull request may close these issues.