-
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 #5308
Conversation
update children transforms before checking their union for culling
…-refactor Cesium 3dtiles inspector refactor
@lilleyse pushing all those sound fine. Please write individual issues for each since some are good beginner issues.
Please do this before the next release.
The main point of this was to aid in reviewing this PR. Let's get some more value out of this by turning it into a short blog post for the Cesium / 3D Tiles community to encourage other 3D Tiles renderers. |
Specs/Scene/Cesium3DTilesetSpec.js
Outdated
@@ -1127,18 +1124,6 @@ defineSuite([ | |||
}); | |||
}); | |||
|
|||
it('replacement and additive refinement', function() { |
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.
This was a duplicate of another test.
Merged master back into here and fixed up the There is some functionality from #5317 that is still missing, like time slicing tiles that are processing and explicitly cancelling requests that go out of view. What's in here is the same behavior that 3d-tiles has always had, with some minor tweaks for using the improved request scheduler. I'll leave the other functionality for a later PR. |
Both of these are important. Please submit a separate issue for each. |
If anyone wants to review or test this again, please do so ASAP so we can merge during the code sprint. |
* @type {Cesium3DTileContent} | ||
* | ||
* @readonly | ||
* @private |
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.
Hi,
we would like to have api access to the original batchTableJson. For this it would be good if the @Private here and at https://github.com/AnalyticalGraphicsInc/cesium/blob/3d-tiles/Source/Scene/Cesium3DTileContent.js#L208
could be removed. Or maybe a direct accessor to the BatchTableJson in the Cesium3DTileContent.js?
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.
Perhaps push this until after this PR to control the scope so we can merge this soon.
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.
yeah after the 3d Tiles merge in master i can create a small PR and get this topic back in the discussion
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.
That would be much appreciated, thanks!
var showExpression = defaultValue(styleJson.show, DEFAULT_JSON_BOOLEAN_EXPRESSION); | ||
var pointSizeExpression = defaultValue(styleJson.pointSize, DEFAULT_JSON_NUMBER_EXPRESSION); | ||
|
||
var expressions = styleJson.expressions; |
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 should be renamed from expressions
to defines
before this 3DTile will become in stable branch to prevent a spec update in next release.
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'll address this after merging this PR and before the next release.
Bump. Just a reminder that let's aim to merge this on Monday morning. |
I tested out a variety of tilesets and I believe this is ready to go. Other Sandcastle example look good too. |
@lilleyse is writing up issues for any remaining task items that we agreed to do post merge. There are a bunch of |
Congratulations, everyone! |
For #3241
With the 3D Tiles spec converging to a 1.0 draft, the
3d-tiles
branch in Cesium has been moving in parallel and is close to being production-ready. I'm opening this PR for some early feedback as we continue to go down the todo list.To do:
RequestScheduler
.CHANGES.md
scene.pickTranslucentDepth = true
?Expression
: Fix eslint in 3D-tiles branch #5372 (comment)