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

Add an isoline feature. #838

Merged
merged 4 commits into from
Jul 12, 2018
Merged

Add an isoline feature. #838

merged 4 commits into from
Jul 12, 2018

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Jun 5, 2018

This adds a Math.log10 polyfill and removes the unused Math.sinh polyfill.

Add simple isoline example.

Add an isoline tutorial. This uses a data set of Oahu's elevation that is lower resolution than the 'dense' set, primarily so that the tutorial will run faster (and test faster).

This resolves #592.

This depends on a variety of open PRs (and includes them as part of this PR) and could be rebased once those are merged it.

@manthey manthey force-pushed the isolines branch 2 times, most recently from 177e838 to 6f690fd Compare June 8, 2018 14:51
@aashish24
Copy link
Member

@manthey is it possible to update this branch?

@manthey
Copy link
Contributor Author

manthey commented Jul 2, 2018

It still depends on the update-jasmine branch. Now that the util-wrap-angle branch is merged, it can be a simple dependency. If you want, I can also collapse all of the commits into a single commit. I didn't want to change it without notice if you were already looking at it.

@manthey manthey changed the base branch from master to update-jasmine July 2, 2018 19:31
@manthey manthey changed the base branch from update-jasmine to master July 9, 2018 17:38
This adds a Math.log10 polyfill and removes the unused Math.sinh
polyfill.

Add simple isoline example.

Add an isoline tutorial.  This uses a data set of Oahu's elevation that
is lower resolution than the 'dense' set, primarily so that the tutorial
will run faster (and test faster).

This resolves #592.

The `used` function can be overridden for isoline and contour features.
This defaults to the `value` function returning a non-null, finite
number.

Pass the calculated position to the `value` function.  This allows the
value to be modified based on the position.

The isoline label feature is added to the list of dependent features so
that something like `myfeature.visible(false).draw()` does what is
expected.

Don't wrap across longitude if the data is more than 360 units wide,
since it probably isn't actually longitude in that case.

Reduce memory use for fully populated meshes.  For large meshes this can
reduce memory use and increase speed.  For small meshes, there is
practically no difference.

Label positions are calculated in map gcs, not interface gcs.  Lines are
not guaranteed to be linear in the interface gcs, so using it may put
labels off of the rendered line.

Modify layers for better nested support.

Layer's z-index should only be relative to their siblings, not all
layers.  Layers don't have to be children of the map; they can be
children of other layers.  Instead of enumerating the map layers for
determining siblings, enumerate the children of the layer's parent,
filtering to only those that are geo.layer instances.

This makes it easier to change the z index of a layer that has dependent
layers (such as the isoline feature's canvas layer for text).
*
* @returns {this}
*/
this.labelPositions = function () {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this as a public function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. By default, we recompute local positions after some amount of zooming or panning. If a user wants the labels to be updated on different criteria, they would call this function.

* labels that need to be shown if there is enough resolution.
*/

/* This includes both the marching triangles and marching squares conditions.
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

* `labelViewport` used in generating labels. The object may have no
* properties if there are no labels.
*/
this.lastLabelPositions = function () {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, not entirely sure if we need this in the public API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. As with labelPositions, this information can be used to decide when to regenerate labels if the default is inappropriate.

}
}
result.index.splice(usedPts);
if (usedPts !== numPts) {
if (i !== numPts) {
Copy link
Member

Choose a reason for hiding this comment

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

whats changed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of the mesh feature was refactored to use less memory by not duplicate arrays if not needed. usedPts is no longer computed while generating a preliminary array, as by skipping that array things are faster. If some vertices are unused, then the previous for loop will have stopped before reaching numPts. In this case we go into the next conditional and exclude the unused vertices.

* If falsy, they will include the exact minimum and maximum values.
* @property {number} [spacing] Distance in value units between isolines.
* Used if specified and `values` is not specified.
* @property {number[]|geo.isolineFeature.valueEntry[]} [values] An array of
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention that either you can use min,max and count or values? I think it is mostly clear, just wondering if we could use bit more strong wording.

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 hope between what is written and the tutorials everything is clear enough. Since values takes precedence, it doesn't mentioned spacing or count, whilst those do mention values.

Prior to this, a custom cmake script (girder.cmake) was used via a
direct URL for each external data file.  After this change, we use
cmake's build-in ExternalData module.  This continues to get all
external data files listed and automatically untar any .tgz files.

External data file references have been moved from testing/test-data to
tests/external-data.  Eventually I'd like there to be only one of the
tests and testing directories, as having two makes things confusing.

This also deletes some abandoned cmake scripts and removes some unused
data files.

As a benefit, data files are now obtained by sha512, which should
guarantee that we have the correct file, even if it changes.  Before,
the download url was used.  This makes it so that we only need one entry
in the repo per data file instead of two.  The only downside is that
ExternalData emits a -hash-stamp file in the download directory.
@manthey manthey merged commit 5cf489c into master Jul 12, 2018
@manthey manthey deleted the isolines branch July 12, 2018 20:35
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.

Add Isolines / Contours with labels
2 participants