three-d icon indicating copy to clipboard operation
three-d copied to clipboard

Pick geometry instance from coordinate

Open thatcomputerguy0101 opened this issue 1 year ago • 1 comments

The current implementation of pick returns the 3d coordinate corresponding to a given 2d coordinate, whereas I'm interested in which geometry or geometry instance of the list corresponds to that 2d coordinate. I've included an example signature below (I'm not entirely sure if the types and lifetimes work correctly)

enum GeometryInstance<G: Geometry> {
    single(G)
    instance(G, u32)
};

fn pick_geometry<'a, G: Geometry>(
    context: &Context,
    camera: &Camera,
    pixel: impl Into<PhysicalPoint> + Copy,
    geometries: impl IntoIterator<Item = &'a G>
) -> Option<GeometryInstance<&'a G>>;

Based on how a depth material is used for regular pick, I believe I would need a material shader that incorporates a geometry ID into the unused color channels. However, as someone who has never worked with shaders before, I was looking for any advice on implementing this.

thatcomputerguy0101 avatar Jul 21 '24 23:07 thatcomputerguy0101

I suppose the existing ColorMaterial could be used instead of another material, and the pick_geometry function allocates solid colors to each provided geometry instance to parse them out later. Might be a performance loss to create said materials every time the geometry is queried, but seems like a much simpler solution to implement that could be later modified for a custom material.

thatcomputerguy0101 avatar Jul 22 '24 03:07 thatcomputerguy0101

That is definitely a missing feature that would be really nice to have. I did the current version that returns the coordinate because that was easiest as the first version, then I never got around to do what you're proposing.

Sounds like you are on the right track. I would create a new material, called IdMaterial or something, that renders an integer given as a uniform to a color. Then for each geometry, I would apply a new IdMaterial with a unique ID, probably the index into the list of geometries. The rest of the setup is very similar to the pick and ray_intersect methods, the method just returns the index into the geometries or the geometry itself.

I think doing this for instances should be possible, but maybe start with regular geometries and see how it goes 🙂

asny avatar Sep 06 '24 10:09 asny

I had a look at the existing code to see how it works (thanks a lot for your work btw), but I don't quite understand why does ray_intersect read the color buffer instead of the depth buffer. Does this provide better precision?

If writing/reading depth from the color buffer is mandatory, can I simply cast some integer geometry id into, say, the remaining 2 color channels (f32) in the shader, then cast back after reading the color buffer?

maxime-tournier avatar Sep 07 '24 15:09 maxime-tournier

I believe I'm fairly close to a working implementation now, including instance detection (which required some slightly jank shader code, especially due to the quirks of MacOS OpenGL and to minimize the number of modified vertex shaders). I've decided to switch the return type over to a struct instead of an enum, with the following signature:

///
/// Representation of a specific geometry instance
///
pub struct GeometryInstance<G: Geometry> {
    /// The base geometry data selected
    pub geometry: G,
    /// The instance id selected, if applicable
    pub instance: Option<usize>,
}

Since I also happen to have depth data, I'm trying to decide if it would ever be useful to also include the intersection position in this return struct.

thatcomputerguy0101 avatar Sep 10 '24 02:09 thatcomputerguy0101

I had a look at the existing code to see how it works (thanks a lot for your work btw), but I don't quite understand why does ray_intersect read the color buffer instead of the depth buffer. Does this provide better precision?

That is a really good question, I think I remember something about the support for reading from the depth buffer was not super great on web - maybe on Safari 🤔 At least the read_depth function in three-d is only available on native, not on web, and there's definitely a reason for that.

If writing/reading depth from the color buffer is mandatory, can I simply cast some integer geometry id into, say, the remaining 2 color channels (f32) in the shader, then cast back after reading the color buffer?

Yes, that is basically it, though I think one channel should be enough. See for example this relevant tutorial.

I believe I'm fairly close to a working implementation now

Nice, make a PR and I'll be happy to review 👍

Since I also happen to have depth data, I'm trying to decide if it would ever be useful to also include the intersection position in this return struct.

Yes, I think that should be there as well. Maybe also the index into the list of geometries.

asny avatar Sep 10 '24 09:09 asny

I think I remember something about the support for reading from the depth buffer was not super great on web

IIRC that was the case under WebGL1 without the WEBGL_depth_texture extension, but I have no idea whether WebGL2 supports it.

See for example this relevant tutorial.

Thanks for the link.

That was my initial approach, however since float32 textures are available I got it working under both desktop/web by directly casting f32s to/from u32s for geometry ids.

See this gist if interested, but in this code object ids are mandatory. I can open a PR if you like.

maxime-tournier avatar Sep 10 '24 10:09 maxime-tournier

IIRC that was the case under WebGL1 without the WEBGL_depth_texture extension, but I have no idea whether WebGL2 supports it.

WebGL2 officially supports reading from a depth render target, but it's not always supported on all browsers anyway 🤷 Might also be that reading f32 from a depth target is not supported. I should really write this type of stuff down, I keep forgetting.

That was my initial approach, however since float32 textures are available I got it working under both desktop/web by directly casting f32s to/from u32s for geometry ids.

Nice, that should work 👍

See this gist if interested, but in this code object ids are mandatory. I can open a PR if you like.

I think object IDs should just be the index given by the order of the geometries iterator passed in. You can always map that outside of the renderer if needed. Otherwise looks good, would be great if you opened a PR 👍 @thatcomputerguy0101 how does this compare to what you have? 🙂

asny avatar Sep 11 '24 08:09 asny

I'm currently troubleshooting a (hopefully final) bug regarding comparing references in an additional example that I created. Other than that, my implementation also creates a new material (which I arbitrarily called GeometryInstanceMaterial), but it also differentiates between multiple copies of InstancedMesh and Particle by utilizing gl_InstanceID and passing it from the geometry/vertex shader to the material/fragment shader.

thatcomputerguy0101 avatar Sep 11 '24 18:09 thatcomputerguy0101

What's confusing me is that printing both references as pointers says that they are same, yet std::ptr::eq returns false when comparing them.

thatcomputerguy0101 avatar Sep 11 '24 18:09 thatcomputerguy0101

Ok, fixed the bugs, doing some cleanup and then I'll create a PR.

thatcomputerguy0101 avatar Sep 11 '24 19:09 thatcomputerguy0101