-
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
Draft for fixing disappearing I3DM models #12105
base: main
Are you sure you want to change the base?
Conversation
Thank you for the pull request, @javagl! ✅ We can confirm we have a CLA on file for you. |
Quick n = 1 test with the windmills with rotor animation: 1] Issue windmill disappear on map navigation (without this PR): Demo: https://bertt.github.io/cesium_issues/i3dm_disappearing2/issue/ 2] Testing this PR Demo: https://bertt.github.io/cesium_issues/i3dm_disappearing2/testing_pr_12105/ Result:
|
Another testcase I see still an instance disappear (the western one) https://bertt.github.io/cesium_issues/i3dm_disappearing2/testing_pr_12105_kast/ GLB used is quite simple: https://bertt.github.io/cesium_issues/i3dm_disappearing2/testing_pr_12105_kast/kast.glb |
Thanks for the quick test and feedback, I really appreciate that 👍 From very quickly skimming/debugstepping over the case with the "blocks" that doesn't work: Yes, in this case, the bounding sphere is still wrong. It doesn't include the second block: I think that this might be related to some of the points that I already mentioned
These are just observations for now, and I haven't yet identified the reason. Admittedly, just transforming the I'll probably need pen+paper to "mathematically confirm" this, but intuitively:
Well ... I could go through this with pen+paper, or just create a test model - and I'm probably lazy and do the latter. EDIT: A minor update: I did check that case of a GLB with translation in an I3DM with all positions being (0,0,0), but different rotations, and the result is as expected: It computes the same bounding sphere for all primitives (with and without the fix from this PR). So... actually fixing this may be a bit more effort. The whole |
Another thing to consider: when the used glTF has larger textures. For example see https://bertt.github.io/cesium_issues/i3dm_disappearing2/issue_textured_trees/ , the used model https://bertt.github.io/cesium_issues/i3dm_disappearing2/issue_textured_trees/tree_textured.glb has 2 textures to visualize the trees |
Btw I'm also wondering what's the purpose of this instancingTranslationMin, instancingTranslationMax compution, as the visibility of the i3dm tile is already defined in tileset.json... |
@bertt I don't understand in how far textures come into play here. I'd think that the issue is independent of the material of the objects...?
I also only started zooming into this. So this attempt to summarize a few things should be taken with a grain of salt:
The computation of the bounding volumes, and how they affect the execution of the draw commands, will have to be revised. In the context of the investigations here, I noticed that the bounding volumes of the models are not computed properly to begin with (regadless of the instancing). This is now tracked in #12108 . So my "quick shot" in this PR will not be sufficient, but my gut feeling is that when ~"the bounding volume computation" is addressed (properly and holistically), the resulting state could also already include a fix for the I3DM issue... |
Description
Under certain conditions, parts of models in I3DM could disappear in certain view configurations. This has been reported in several forum threads. Most recently, in https://community.cesium.com/t/disappearing-i3dms-on-map-navigation-again/34035 . It is currently tracked in #11176.
A short ping @bertt : Maybe you want to try this out. It's not yet tested extensively, but might fix the issue.
For the reviewers:
Some details from investigating the reason for the error are summarized in #11176 (comment)
The attempt of a tl;dr summary:
boundingVolume
that is associated with aDrawCommand
determines whether that command is executed or not (depending on the view configuration)PrimitiveRenderResources.positionMin
/positionMax
properties. ThesepositionMin/positionMax
properties already include the instancing translationFor nodes with different transforms, this had to be inconsistent.
This PR is currently a DRAFT state, also because it contains some debug output. Some of this output - from the example that was posted at the bottom of #11176 (comment) - is shown here to convey the idea. It is an I3DM with 4 instances of a GLB that contains 4 primitives with different sizes that are attached to nodes with different scaling factors.
Before the fix, the output is this:
The
instancingTranslationMin/Max
are always the same. They are determined directly from the instancing information.The computed
positionMin/Max
show differences that depend on thenode scale ...
The most obvious (and relevant) effect is on the bounding shere radius:
Shown in this screenshot:
I considered different approaches for fixing this. One difficulty is...
positionMin/Max
do not say whether they are in world space or object spaceinstancingTranslationMin/Max
does not say whether it is in world space. But there is a subtle hint viaModelComponents/Instances.transformInWorldSpace
which says that it can be both...The suggested fix is summarized in this comment.
It's a bit... pragmatic, and having to compute the inverse of the node transform is ... not so nice. But other approaches had a larger potential for messing up other uses of the
positionMin/Max
(for which I now added a few lines of comments, by the way).With this fix, the log output is as follows:
One can see that the
positionMin/Max
now all have to be transformed with the node transform, but then yield the actual, correct result - and it computes the same bounding sphere radius for all cases. The result is shown here:(Yes, I see that the bounding sphere does not really include the whole model - where the mouse cursor is. Should I fix this? Is this my fault? We can talk about that...)
Issue number and link
Addresses #11176
Testing plan
What remains to be investigated is the effect of other instancing transforms. Right now, the transforms only contain translations. I would expect unexpected effects when the instancing transforms contain additional rotations ot scaling. (But... maybe not, if the
instanceTranslationMin/Max
are already computed properly and taking this into account 🤞 )For testing: Locally load the tileset (+Sandcastle) that are attached at the bottom of #11176 (comment) , and check whether there is still a view configuration where parts of the model disappear. Maybe also try out some of the examples that had been posted in https://community.cesium.com/t/disappearing-i3dms-on-map-navigation-again/34035 (that's also still on my TODO list)
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change(This is a DRAFT state with debug/log output and TODO comments - only for an early review)