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

DebugTilesPlugin: add the displayParentBounds option (#730) #893

Merged

Conversation

sguimmara
Copy link
Contributor

@sguimmara sguimmara commented Dec 23, 2024

This PR fixes #730.

Adds the .displayParentBounds property to display the bounds of tiles between the root tile and the first visible tile.

The helpers for the intermediate bounds use a thicker line and semi-transparent materials to help distinguishing them from the "leaf" tiles.

image

Note: not all GPUs support thick lines (see the comment in the three.js doc), so on some platforms the line will remain 1-pixel thick. The alternative would be to use different materials, but that would increase complexity and maintenance.

This uses a reference counting mechanism as suggested in #730.

@sguimmara
Copy link
Contributor Author

@gkjohnson the default example is not ideal to test it since the child bounds tightly fit in the parent bounds. Do you have an example where there is more loose space between parent/child bounds ?

@sguimmara sguimmara force-pushed the debug-tiles-plugin-parent-bounds branch from 337a79a to 86c9b4d Compare December 23, 2024 12:36
@sguimmara sguimmara marked this pull request as draft December 23, 2024 13:18
@sguimmara
Copy link
Contributor Author

@gkjohnson the default example is not ideal to test it since the child bounds tightly fit in the parent bounds. Do you have an example where there is more loose space between parent/child bounds ?

Nevermind I just realized that we could pass arbitrary URLs in the example

@sguimmara sguimmara force-pushed the debug-tiles-plugin-parent-bounds branch 2 times, most recently from 7819583 to 45c6181 Compare December 23, 2024 13:36
@sguimmara sguimmara marked this pull request as ready for review December 23, 2024 13:36
Copy link
Contributor

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

Thanks - a couple comments. I'm hoping this can be used to help understand what's happening in #844.

As expected it can look a bit messy but this is great progress

image

Comment on lines 683 to 682
material.linewidth = 4;
material.opacity = 0.2;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to avoid using line width so we're consistent between platforms - it's not working on my machine, for eample. As you say it looks like we'll need a better treatment for these lines, though, so they're more distinguishable. The random colors for each depth tier don't really help, either. But I think we can evolve this as it's actually used for debugging stuff - the logic for toggling parents is great to have added, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the line width change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the top of my heads, here are a few ideas to help visualize the relationship between parent and child bounds:

  • optionally connecting lines between the center of the parent bounds to the centers of child bounds, the result would look like some kind of "nervous system", that would help figuring the structure of the hierarchy
  • using materials that support dashes and use dashed lines for intermediate bounds
  • optionally, when hovering a tile (using raycasting), display only the bounds of the sub-tree that this tile belongs to, up to the root tile

Copy link
Contributor

Choose a reason for hiding this comment

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

optionally, when hovering a tile (using raycasting), display only the bounds of the sub-tree that this tile belongs to, up to the root tile

This one I think would probably be the most useful. I think the other visual treatments for the parent or child lines will be really noisy and a bit difficult to understand. I'll merge this for now and we can consider this in the future

Comment on lines -84 to +89
// compute the vertex normals so we can get the edge normals
geometry.computeVertexNormals();
if ( computeNormals ) {

// compute the vertex normals so we can get the edge normals
geometry.computeVertexNormals();

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When profiling a tileset with region bounds, I noticed that the biggest performance hit was computing normals for ellipsoid helpers. However, the line-based variant of the helper do not need normals AFAIK.

@sguimmara sguimmara force-pushed the debug-tiles-plugin-parent-bounds branch from 45c6181 to b4bceeb Compare December 24, 2024 09:01
@gkjohnson
Copy link
Contributor

Very nice work! Thanks for the addition

@gkjohnson gkjohnson merged commit 07af9a7 into NASA-AMMOS:master Dec 24, 2024
2 checks passed
@sguimmara sguimmara deleted the debug-tiles-plugin-parent-bounds branch December 24, 2024 16:35
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.

DebugTilesPlugin: Add support for displaying parent bounds
2 participants