-
Notifications
You must be signed in to change notification settings - Fork 370
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
Implement outlines for points 2d/3d/depth & use them for select & hover in Viewer #1568
Conversation
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.
🥳
@@ -13,6 +13,8 @@ var color_texture: texture_2d<f32>; | |||
struct BatchUniformBuffer { | |||
world_from_obj: Mat4, | |||
flags: u32, | |||
_padding: UVec2, // UVec3 would take its own 4xf32 row, UVec2 is on the same as flags |
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.
why do we need to pad at all?
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.
It's either here or in Rust. If we put nothing here they get on the same "row" since alignment for UVec2 is 8. https://www.w3.org/TR/WGSL/#alignment-and-size
This then translates to
pub struct BatchUniformBuffer {
pub world_from_obj: wgpu_buffer_types::Mat4,
pub flags: u32, // PointCloudBatchFlags
pub outline_mask_ids: glam::Vec2,
pub end_padding: [wgpu_buffer_types::PaddingRow; 16 - 5],
}
But bytemuck
doesn't like this because that implies 32bit padding after outline_mask_ids
and it forces all padding to be explicit. So we end up with
pub struct BatchUniformBuffer {
pub world_from_obj: wgpu_buffer_types::Mat4,
pub flags: u32, // PointCloudBatchFlags
pub outline_mask_ids: glam::Vec2,
pub _padding: u32,
pub end_padding: [wgpu_buffer_types::PaddingRow; 16 - 5],
}
Migrates the last primitives, points 2d/3d & depth cloud, over to outline based selection. Closes #889, from here on out there is still cleanup and more improvements to make, but we got where we wanted to be in rough strokes!
Some of the cleanup already happens here because of unused-warnings, but there's a bit more to do.
Clip demonstrating what it looks like as well as what it looks like for small & large points. Unsurprisingly it works out best for connected points.
https://user-images.githubusercontent.com/1220815/224499438-abf2bba4-7262-414e-88c7-65d25f0014ea.mov
(yes it also works for 2D, not dmonstrated here because as time of recording that wasn't done yet)
Approach on single points is identical to #1553
Note that depth cloud didn't have any selection highlight before (this pr naturally doesn't fix picking for it and doesn't allow selecting single points for it either)
Unsurprisingly this also speeds up point processing! Nowhere near the depth cloud feature and not nearly enough to close issue #850, but a fair bit!
For the the usual unscientific nyud demo testscenario (very long point history, loop a bunch of frames at end) I'm getting 2.5ms for free in release mode, mostly in color processing.
Before:
After:
Checklist