Update styles from arrays. #687
Conversation
This allows a convenient way to animate across styles, and also provides a way where, if appropriate, rerendering can be optimized based on just what has changed. Per-vertex styles can be set on lines and polygons by passing an array of arrays. Some styles on points and contours were being queried via functions of the form styleFunc(data) where they should have been queried as styleFunc(data, index). These have been changed. Speed up paths for other features will be added later.
manthey
force-pushed
the
style-from-array
branch
from
117d08f to
0530e36
Compare
|
This resolves issue #683. Optimizations for lines and polygons can be done later. Optimizations for positions will take more work. |
Codecov Report
@@ Coverage Diff @@
## master #687 +/- ##
==========================================
+ Coverage 95.22% 95.28% +0.05%
==========================================
Files 84 84
Lines 8920 8990 +70
==========================================
+ Hits 8494 8566 +72
+ Misses 426 424 -2
Continue to review full report at Codecov.
|
src/feature.js
Outdated
| * value per data item. The values are not converted or validated. Color | ||
| * values should be objects with r, g, b values on a scale of [0, 1]. If | ||
| * invalidate values are given the behavior is undefined. | ||
| * For features where this._subcomponentStyles is an object and a style |
There was a problem hiding this comment.
Since this is a public API, listing private API (from documentation point of view) could be confusing to the user.
There was a problem hiding this comment.
I'll improve the comment.
src/feature.js
Outdated
| } else { | ||
| var fallback; | ||
| if (keyOrObject.toLowerCase().match(/color$/)) { | ||
| fallback = {r: 0, g: 0, b: 0}; |
There was a problem hiding this comment.
Should be define these defaults as someplace else where it is more visible?
There was a problem hiding this comment.
This is the only one, because colors are always coerced into the rgb object and other functions except them not to have undefined values. Moving it elsewhere will just obscure what happens, I think.
There was a problem hiding this comment.
Sure I am fine with being here for now and we can revisit if needed to be in the future.
| @@ -60,7 +62,7 @@ var gl_pointFeature = function (arg) { | |||
| ' precision highp float;', | |||
| '#endif', | |||
| 'attribute vec3 pos;', | |||
| 'attribute float rad;', | |||
| 'attribute float radius;', | |||
src/feature.js
Outdated
| @@ -44,6 +44,10 @@ var feature = function (arg) { | |||
| m_dependentFeatures = [], | |||
| m_selectedFeatures = []; | |||
|
|
|||
| // subclasses can add keys to this for styles that apply to subcomponents of | |||
| // data items, such as individual vertices on lines or polygons. | |||
| this._subcomponentStyles = {}; | |||
There was a problem hiding this comment.
should we just call it componentStyles? or perFeatureStyle? sub and component in my understanding convey very similar message.
There was a problem hiding this comment.
It really is intended to be part of the private API, so it should only matter to developers. This is the list of styles that are per-vertex rather than per-feature when there is a distinction between vertices and features (i.e., not on point features). There could be a feature where the sub-feature is not a vertex but something else, so this isn't strictly a list of vertex styles. I don't know what a better name would be. perFeatureStyle is definitely wrong, as these apply at a finer granularity than features. _subfeatureStyles might be better. _stylesAtFinerGranularityThanPerFeature is too long.
This allows a convenient way to animate across styles, and also provides a way where, if appropriate, rerendering can be optimized based on just what has changed.
Per-vertex styles can be set on lines and polygons by passing an array of arrays.
Some styles on points and contours were being queried via functions of the form styleFunc(data) where they should have been queried as styleFunc(data, index). These have been changed.
Speed up paths for other features will be added later.