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

Vector tiles polygons #4186

Merged
merged 91 commits into from
Nov 18, 2016
Merged

Vector tiles polygons #4186

merged 91 commits into from
Nov 18, 2016

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Aug 9, 2016

Adds vector tile support and a custom ground primitive for rendering polygons of a vector tile.

Both polygons an polylines:

  • Doc
  • Per feature properties
  • Styling
  • Doc
  • Unit tests
  • Update 3D Tiles Sandcastle example (could be just one vector example)
  • Z-ordering?

Polygons:

  • Option to outline polygons.
  • Picking issue where sometimes the color bleeds to other volumes
  • Move work in createVertexArray to a Web Worker.
  • Compute tighter OBBs for picking.

Polylines:

  • Investigate polygon offset values
  • Investigate incomplete polyline rendering issues (more likely an issue with the tileset)
  • Render during GROUND pass?

bagnell added 30 commits June 16, 2016 16:00
var count = batchedIndices[j / 3].count;

// stencil preload command
var command = commands[j];
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid reusing the same command variable here for three different commands? Just name each one based on the pass.

var bv = primitive._boundingVolumes[j / 3];

// stencil preload command
var command = pickCommands[j];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment.


/**
* Colors the entire tile when enabled is true. The resulting color will be (polygon batch table color * color).
* @private
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the class is marked private, there's no need to mark everything else private.

* Part of the {@link Cesium3DTileContent} interface.
*/
Vector3DTileContent.prototype.getFeature = function(batchId) {
var featuresLength = this.featuresLength;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can go inside the //>>includeStart...

attribute vec3 currentPosition;
attribute vec3 previousPosition;
attribute vec3 nextPosition;
attribute vec2 expandAndWidth;
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about spreading this and a_batchId across the w components of current, previous, and next?

float width = abs(expandAndWidth.y) + 0.5;
bool usePrev = expandAndWidth.y < 0.0;

vec4 p = u_modifiedModelView * vec4(currentPosition, 1.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't do the above, still declare them as vec4 and w will be implicitly 1.0, which will clean this up.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 12, 2016

Can you add more fine-grained unit tests for the batched polyline and polygon?

Otherwise, how is code coverage?

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 12, 2016

I did not run this (I will when everything is done), but that is all my comments. Mostly trivial.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 18, 2016

@pjcozzi This is ready for another look. I merged in to code for points on terrain and styling them like points/billboards/labels.

The original branch strategy was to have branches for each of the features (polygons/polylines/points), but that was a mess. Everything is now in this branch, so I'll merge it into vector-tiles and open a PR into 3d-tiles-vector which will be the master for vector tiles. Any other PRS will be into that branch.

@bagnell bagnell merged commit 84eb86c into vector-tiles Nov 18, 2016
@bagnell bagnell deleted the vector-tiles-polygons branch November 18, 2016 00:48
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