search

asny / three-d

2D/3D renderer - makes it simple to draw stuff across platforms (including web)
MIT License
1.33k stars 110 forks source link

Return geometry instance when picking #478

Closed maxime-tournier closed 2 months ago

maxime-tournier commented 2 months ago

Also add a new material, IdMaterial that renders depth and index for each geometry.

Closes https://github.com/asny/three-d/issues/467

thatcomputerguy0101 commented 2 months ago

Your IdMaterial currently has the same id as DepthMaterial, which would break if those two materials were ever used in the same program due to how shaders are cached by their id.

maxime-tournier commented 2 months ago

Your IdMaterial currently has the same id as DepthMaterial, which would break if those two materials were ever used in the same program due to how shaders are cached by their id.

Good catch, thanks a lot. This means my PR would break e.g. shadow maps, right? It's unclear to me how to properly choose an available ID though...

I see your PR handles shader ID management automatically and also detects instance ID, so it should definitely be favored over my attempt (unless hardcoded IDs are somehow desirable?).

Maybe yours could be split in two PRs, one for ID management only, and another for picking geometry instance?

thatcomputerguy0101 commented 2 months ago

Yes, as-is, this PR would break shadow maps. Figuring out what IDs were available were what led me to restructuring the ID system in my PR. I don't intend to split the PR up unless that's what the maintainer wants due to how the changes depend on each other.

maxime-tournier commented 2 months ago

Alright, let's clone this one then. Thanks for your feedback!

asny commented 2 months ago

Thanks both for implementing this 🙏 I never experienced two people doing the same feature at the same time before 🚀