-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
3D Tiles doc fixes #5356
3D Tiles doc fixes #5356
Conversation
Apps/CesiumViewer/CesiumViewer.js
Outdated
@@ -34,7 +34,7 @@ define([ | |||
* 'debug' : true/false, // Full WebGL error reporting at substantial performance cost. | |||
* 'lookAt' : CZML id, // The CZML ID of the object to track at startup. | |||
* 'source' : 'file.czml', // The relative URL of the CZML file to load at startup. | |||
* 'stats' : true, // Enable the FPS performance display. | |||
* 'statistics' : true, // Enable the FPS performance display. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just changed to statistics
everywhere so that i wouldn't miss anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should change Viewer
, no need to create a breaking change for this now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few small comments.
Apps/CesiumViewer/CesiumViewer.js
Outdated
@@ -34,7 +34,7 @@ define([ | |||
* 'debug' : true/false, // Full WebGL error reporting at substantial performance cost. | |||
* 'lookAt' : CZML id, // The CZML ID of the object to track at startup. | |||
* 'source' : 'file.czml', // The relative URL of the CZML file to load at startup. | |||
* 'stats' : true, // Enable the FPS performance display. | |||
* 'statistics' : true, // Enable the FPS performance display. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should change Viewer
, no need to create a breaking change for this now.
CHANGES.md
Outdated
@@ -10,6 +10,8 @@ Change Log | |||
* `Cesium3DTileContent` | |||
* `Cesium3DTileFeature` | |||
* `Cesium3DTilesInspector`, `Cesium3DTilesInspectorViewModel`, and `viewerCesium3DTilesInspectorMixin` | |||
* `Cesium3DTileColorBlendMode` | |||
* `Cesium3DTileRefine` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remind me why this is public? Because it is on a getter in Cesium3DTile
? What is the use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a use case, I'll make private again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's a getter on Cesium3DTile
which I can make private too,
Source/Scene/Cesium3DTileContent.js
Outdated
@@ -227,10 +228,13 @@ define([ | |||
* given <code>batchId</code>. This object is used to get and modify the | |||
* feature's properties. | |||
* | |||
* Features in a tile are ordered by <code>batchId</code>, an index used to retrieve their metadata from the batch table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be surrounding with <p></p>
.
Updated. |
From comments in #5308