Generalize meshes #817
Generalize meshes #817
Conversation
manthey
force-pushed
the
generalize-mesh
branch
5 times, most recently
from
7533822 to
6a54944
Compare
|
@aashish24 This now has tests for all of the new code. |
Generalize the rectangular grid used in the contour feature to a mesh that could also be composed of enumerated triangular or square elements. This adds more tests to the contour feature and fixes some jsdocs. The mesh mixin is not fully tested yet.
src/meshFeature.js
Outdated
| * data point is removed from the resultant mesh. | ||
| * @returns {geo.meshFeature.meshInfo} An object with the mesh information. | ||
| */ | ||
| this.createMesh = function (vertexValueFuncs) { |
There was a problem hiding this comment.
@manthey does this needs to be in public API?
There was a problem hiding this comment.
It doesn't need to be. My reasoning for it to be a public API, is that making so encourages its reuse in other components.
There was a problem hiding this comment.
I see but from the end-user perspective, I do see rare use specially since this does not seem like a self contained method. I would make it _createMesh but if you have strong feelings about keeping it public, I am fine with that.
| * @property {number|function} [style.opacity=1] The opacity on a scale of 0 to | ||
| * 1. | ||
| * @property {geo.geoPosition|function} [style.position=data] The position of | ||
| * each data element. This defaults to just using `x`, `y`, and `z` |
There was a problem hiding this comment.
very minor document issue (not sure if that matters when this will get rendered). Seems to have lot many blank spaces between the '.' and 'This'.
There was a problem hiding this comment.
There are two popular choices in typography, two spaces between sentences or one. There are impassioned arguments for each side, and, since it is opinion, there is no strict right or wrong. I use two spaces. HTML is poor in that it collapses all spaces to a single space, losing the semantic difference between sentence separation and word separation, but in the source, this is still apparent. In old typography manuals, it used to be argued that word separation should be 1/5 em and sentence separation should be 1/3 em, but that doesn't work in monospaced fonts.
There was a problem hiding this comment.
I see, I am fine with this as long as we are consistent. We could probably mention this somewhere so that a new developer follows the same.
There was a problem hiding this comment.
We need to update the docs in general to mentioned all of the desired behavior for jsdocs in general.
The function can take optional bounds to override the calculated values, and can take the smaller spanning range of the specified bounds and the calculated bounds.
Move calculating the min and max of an array to a utility function.
Generalize the rectangular grid used in the contour feature to a mesh that could also be composed of enumerated triangular or square elements.
This adds more tests to the contour feature and fixes some jsdocs.