Skip to content

Option to update vector layers while animating #2385

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

Merged
merged 1 commit into from
Jan 14, 2015

Conversation

ahocevar
Copy link
Member

With this option, vector batches will be recreated with every frame.
For animations, this means that vector data won't be clipped to the
extent at which the animation started.

@tschaub
Copy link
Member

tschaub commented Jul 15, 2014

I think this can be handled separately, but what are your thoughts on making this options available in another way? It feels more natural to have rendering related options (rather than calling them device options). Should this be map level or layer level?

@@ -54,6 +55,14 @@ olx.DeviceOptions.prototype.loadTilesWhileInteracting;


/**
* When set to true, vector replay batches will be recreated with every
* animation frame, which has a performance impact for large vector layers.
* Default is `false`.
Copy link
Member

Choose a reason for hiding this comment

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

Missing @type annotation.

@ahocevar
Copy link
Member Author

I'm open to making this available in a different way, but haven't thought of different ways. This pull request may not even be needed - at least for now I used a FeatureOverlay instead, which gives me the desired behavior out of the box.

@tschaub
Copy link
Member

tschaub commented Jul 15, 2014

Personally, I'm in favor of exposing rendering options assuming we can provide alternate strategies without the code becoming a mess (which is not happening here). I think people will likely develop applications and then discover that they want slightly different behavior - less aggressive tile loading, more frequently updated vector rendering, etc. It will be far easier to modify existing application code by adding rendering related options instead of using entirely different objects (swapping out a vector layer and a vector source for a feature overlay for example).

@elemoine
Copy link
Member

Agree @tschaub. In this case I think it should be a vector layer option.

@ahocevar
Copy link
Member Author

I updated this pull request by adding an updateVectorsWhileAnimating option on the map. I think this is in line with the loadTilesWhileAnimating and loadTilesWhileInteracting options, which we also have on the map.

@elemoine
Copy link
Member

Previously, we said that would be a per-layer option. What is the reason for using a map option instead?

@ahocevar
Copy link
Member Author

@elemoine: you said that. @tschaub was just asking. The reason was to make it work the same as loadTilesWhileAnimating. But other than with tiles (which are managed by the map), it would be possible to make it a layer setting. It would also be possible to have both: layer setting overrides map setting.

@ahocevar
Copy link
Member Author

I'll also make this work with the other renderers. @elemoine: do you want it on both map and layer, or just layer? You wish we code 😄.

@elemoine
Copy link
Member

I just think that a layer option would be more flexible.

@ahocevar
Copy link
Member Author

Cool. Will change.

@ahocevar ahocevar force-pushed the update-vectors-while-animating branch from c97e6db to 4ba4555 Compare January 14, 2015 13:20
@ahocevar
Copy link
Member Author

Pull request updated. updateWhileAnimating is now a layer option.

@fredj
Copy link
Member

fredj commented Jan 14, 2015

LGTM, thanks Andreas

* This means that no vectors will be shown clipped, but the setting will have a
* performance impact for large amounts of vector data. When set to `false`,
* batches will be recreated when no animation is active. Default is `false`.
* @type {boolean|undefined}
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need an @api marker here?

With this option, vector batches will be recreated with every frame.
For animations, this means that vector data won't be clipped to the
extent at which the animation started.
@ahocevar ahocevar force-pushed the update-vectors-while-animating branch from 4ba4555 to b1a73da Compare January 14, 2015 15:33
@elemoine
Copy link
Member

Please merge.

ahocevar added a commit that referenced this pull request Jan 14, 2015
Option to update vector layers while animating
@ahocevar ahocevar merged commit c726cbf into openlayers:master Jan 14, 2015
@ahocevar ahocevar deleted the update-vectors-while-animating branch January 14, 2015 16:34
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.

4 participants