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

Data source outline width #2255

Merged
merged 13 commits into from
Nov 6, 2014
Merged

Data source outline width #2255

merged 13 commits into from
Nov 6, 2014

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Nov 5, 2014

The bulk of this change adds geometry outline width support to the DataSource layer. This is exposed via a new outlineWidth property on EllipseGraphics, EllipsoidGraphics, PolygonGraphics, RectangleGraphics, and WallGraphics. For non-time-dynamic data, StaticOutlineGeometryBatch now groups geometry by width before splitting them into translucent/opaque groups. Time-dynamic data regenerates geometry every frame as it was already doing (but now includes outline width).

Also:

  • Added outlineWidth support to CZML geometry packets.
  • Added stroke-width support to the GeoJSON simple-style implementation.
  • Fixed external Khronos links in documentation.
  • Fixes some bad tests for the above graphics classes and also added missing variable declaration from their constructor functions (and of course add tests for the new functionality).
  • Also added a private function Scene.clampLineWidth, which clamps the value to line widths supported by the underlying WebGL implementation.

Should we add information somewhere regarding browsers only supporting a line width of 1 on Windows? I'm sure it will continue to be a common question on the mailing list; more so now that we're exposing it at a higher level.

@@ -2334,6 +2334,10 @@ define([
return new PickId(this._pickObjects, key, Color.fromRgba(key));
};

Context.prototype.putLineWidthInValidRange = function(width) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Context is private. We try to avoid using it unless we are in rendering code. Perhaps make this Scene.clampLineWidth even if we don't want to make it public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, wasn't sure if Scene or Context was the more appropriate place. I'll leave it private for now, we can always expose it later if we need to.

Move and rename `Context.putLineWidthInValidRange` to `Scene.clampLineWidth`
@mramato mramato mentioned this pull request Nov 5, 2014
70 tasks
@mramato
Copy link
Contributor Author

mramato commented Nov 5, 2014

I forgot to add that the only way to test this on Windows is to use Chrome Canary with the --use-gl=desktop command line argument. (Stable and beta won't work). Firefox may work if you toggle the native webGL option in about:config, but on my machine it just broke WebGL.

@mramato mramato mentioned this pull request Nov 5, 2014
Only depth test opaque outlines, move outline primitives last to be drawn.
@mramato
Copy link
Contributor Author

mramato commented Nov 6, 2014

@bagnell Had some offline comments which I've now made. This is ready for another look or to be merged.

bagnell added a commit that referenced this pull request Nov 6, 2014
@bagnell bagnell merged commit aa1c76c into master Nov 6, 2014
@bagnell bagnell deleted the dataSource-outlineWidth branch November 6, 2014 22:50
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.

3 participants