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

Update styles from arrays #683

Closed
manthey opened this issue Mar 29, 2017 · 17 comments
Closed

Update styles from arrays #683

manthey opened this issue Mar 29, 2017 · 17 comments

Comments

@manthey
Copy link
Contributor

manthey commented Mar 29, 2017

Previously, we've animated line and point features by getting access to the WebGL Float32Arrays used for specific styles, such as opacity or color, modified those arrays, and then rerendered. This is very fast, but requires knowledge of internal data structures. For unclustered points, for instance, each point has a a record that has some number of vertices per point (which could be 1, 3, or 4 depending on how we constructed the point feature). The user has to know to use this value.

As an alternative, if we added a function to set styles from arrays, the generic mode would just update that style by converting the array to a function:

this.styleFromArray = function (key, styleArray, refresh) {
  this.style(key, function (d, i) {
    return styleArray[i];
  });
  if (refresh && this.visible()) {
    this.draw();
  }
  return(this);
}

The styleArray contents in this example would be not be functions. This means for lines and polygons, there would be one value for each line or polygon regardless of how many line segments or vertices they had.

Perhaps this could also take a dictionary of style keys and their associated arrays as a convenience to avoid having to call the function multiple times for multiple concurrent style changes.

For features where we chose to optimize things, the subclass could provide an extra method to accelerate updates. For instance, in the gl_pointFeature, this function could be:

s_styleFromArray = <parent>.styleFromArray;
this.styleFromArray = function (key, styleArray, refresh) {
  if (this.visible() {
    var vpf, mapper, buffer, numPts, value, i, j, v;
    switch (key) {
      case 'fillOpacity': case 'strokeOpacity':
        numPts = this.data().length;
        mapper = this.actors()[0].mapper();
        buffer = mapper.getSourceBuffer(key);
        vpf = this.verticesPerFeature();
        if (!buffer || !numPts || numPts * vpf !== buffer.length) {
          break;
        }
        for (i = 0, v = 0; i < numPts; i += 1) {
          value = styleArray[i];
          for (j = 0; j < vpf; j += 1, v += 1) {
            buffer[v] = value;
          }
        }
        mapper.updateSourceBuffer(key);
        // I'll have to check if this causes a refresh or not, but we don't need to refresh from the superclass call
        refresh = false;
        break;
    }
  }
  // we always need to update the style to keep the overall state consistent.
  return s_styleFromArray(key, styleArray, refresh);
}

There are a few dangers to this optimization: (a) the values extracted from styleArray are not checked for validity, so passing bad values will do odd things, (b) for efficiency some styles will be necessarily more restrictive than they are in general. For instance, colors would need to be in {r: , g: , b: } rather than our permissive css possibly with opacity processing we currently do, (c) it may not work with certain features, such as point clustering, (d) this doesn't give access to per-vertex values of lines and polygons. All of this could be worked around, but for animations, speed is paramount, so any checks and validation need to be very quick -- for instance, we could skip the fast path if points are clustered.

Thoughts?

@aashish24
Copy link
Member

aashish24 commented Mar 30, 2017

Perhaps this could also take a dictionary of style keys and their associated arrays as a convenience to avoid having to call the function multiple times for multiple concurrent style changes.

+1

@aashish24
Copy link
Member

s_styleFromArray = .styleFromArray;
this.styleFromArray = function (key, styleArray, refresh) {

LRTM

@aashish24
Copy link
Member

(a) the values extracted from styleArray are not checked for validity, so passing bad values will do odd things,

I think thats fine and as discussed we can document this. As you stated, while animating, the end-users has responsibility to make sure their data is correct. As far as we can generate reasonable error messages (**whenever possible because in some cases the error may be from GL in which case we cannot do much about it).

@aashish24
Copy link
Member

For instance, colors would need to be in {r: , g: , b: } rather than our permissive css possibly with opacity processing we currently do,

Do we do that? or should be ask users to give us an array of values?

@aashish24
Copy link
Member

(d) this doesn't give access to per-vertex values of lines and polygons.

I think that should be fine. Per vertex thing would be for users who knows graphics and programming at a deeper level. For them then can grab the GL bufffers I would think.

@aashish24
Copy link
Member

but for animations, speed is paramount

Yes, 15 FPS min +1

@manthey
Copy link
Contributor Author

manthey commented Mar 30, 2017

We run style color values through a function that returns an object with r, g, b values on a scale of [0, 1]. In a few locations we will also use an a value for opacity (but not everywhere). If an array for color is an array of objects with r, g, b, it will behave as expected. Anything else will require additional conversion (and therefore reduce speed).

@aashish24
Copy link
Member

We run style color values through a function that returns an object with r, g, b values on a scale of [0, 1]. In a few locations we will also use an a value for opacity (but not everywhere). If an array for color is an array of objects with r, g, b, it will behave as expected. Anything else will require additional conversion (and therefore reduce speed).

I see, I think for now for consistence {r:, g:, b:} is fine.

i am wondering if we could call this method updateStyleFromArray or just simply updateStyle? but I am also fine with styleFromArray, my only thinking is I feel like that styleFromArray missing the impact that this is meant for a dynamic runtime change vs a initial call

@aashish24
Copy link
Member

Other than that I think I am good with overall idea. We could probably file a bug for supporting animations of position as well if it is not there already.

@manthey
Copy link
Contributor Author

manthey commented Mar 30, 2017

updateStyleFromArray sounds fine to me.

@manthey
Copy link
Contributor Author

manthey commented Mar 30, 2017

Animating positions is intrinsically slower than other properties due to coordinate systems. The data is in the feature.gcs coordinate system (which is map.ingcs by default), and is often latitude and longitude. It has to be converted to the map.gcs coordinate system, which is often web Mercator. Even if the style array is provided in map.gcs, for consistency, we'd want to update the actual style with values in feature.gcs, so a back-conversion would then be necessary.

As a complication, lines and polygons have coordinates per vertex (necessarily), so updating the style via an array per data item doesn't work there, either.

A function (in the future) could be written to handle this, but the interface is a lot less obvious -- should this now be an array of arrays (probably)? or a single concatenated array. Polygons with holes are complicated.

Again, we can expose speed-ups on a per feature basis -- points could allow updating geometry via an array, and the coordinate system conversion would cost some time, but it would still be faster than the current method. Other features would simply not support updating position by array for now.

@aashish24
Copy link
Member

Animating positions is intrinsically slower than other properties due to coordinate systems. The data is in the feature.gcs coordinate system (which is map.ingcs by default), and is often latitude and longitude. It has to be converted to the map.gcs coordinate system, which is often web Mercator. Even if the style array is provided in map.gcs, for consistency, we'd want to update the actual style with values in feature.gcs, so a back-conversion would then be necessary.

roger that

@aashish24
Copy link
Member

As a complication, lines and polygons have coordinates per vertex (necessarily), so updating the style via an array per data item doesn't work there, either.

right. And actually for lines I would think that we will have to be bit more creative. I see a level in animation based on positions:

  1. Points - Plain simple animations where points can move in the coordinate space to showcase physical behavior

  2. Lines - A track created by a car or hurricane is moving, perhaps requires some concept of trail and timeout for the trail

  3. Polygons - I am not entirely clear while writing this a good use-case for this in geospatial context.

@aashish24
Copy link
Member

aashish24 commented Mar 30, 2017

A function (in the future) could be written to handle this, but the interface is a lot less obvious -- should this now be an array of arrays (probably)? or a single concatenated array. Polygons with holes are complicated.

I agree +1

@aashish24
Copy link
Member

Again, we can expose speed-ups on a per feature basis -- points could allow updating geometry via an array, and the coordinate system conversion would cost some time, but it would still be faster than the current method. Other features would simply not support updating position by array for now.

Make sense, points would be most obvious and doable for now.

@manthey
Copy link
Contributor Author

manthey commented Mar 31, 2017

I've got a fist pass of this working on my local machine, with just the pointFeature optimized for radius, fill, fillOpacity, stroke, strokeOpacity, and strokeWidth. Initially, while animating 4 styles (radius, fillOpacity, strokeOpacity, and strokeWidth) with 10% of the points having any opacity in any given frame, I get 60 fps for 500,000 points, 30 fps for 1,000,000 and 15 fps for 2,000,000.

@manthey
Copy link
Contributor Author

manthey commented Jun 1, 2017

Resolved by PR #687.

@manthey manthey closed this as completed Jun 1, 2017
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

No branches or pull requests

2 participants