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

Added per-instance visibility attribute to the instancing_modified example #22147

Closed
wants to merge 1 commit into from

Conversation

WestLangley
Copy link
Collaborator

As suggested by @mrdoob.

TBH, I think the demo needs improvement.

I'd be just as happy to remove it, but I think the code-injection pattern is informative.

@WestLangley
Copy link
Collaborator Author

WestLangley commented Aug 9, 2021

I don't like it either, but perhaps someone can suggest a better demo.

@WestLangley WestLangley deleted the dev-instancing-modified branch August 16, 2021 13:33
@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 10, 2022

Sorry for the late feedback but how about controlling the instance visibility like so:

  • Maintain an array of 3D objects with a custom visibility property in userData. These "dummy" 3D objects represent the instances.
  • Set the transform, color and visibility.
  • Sort instances in the order visible/not visible and update the attributes of InstanceMesh accordingly.
  • Use InstancedMesh.count to determine the number of instances that should be rendered (determining the draw range).

The downside of this approach is you need a 3D object per instance to keep track of the instance state. However, it's not necessary to discard fragments so you only render what's actually necessary. Overall, it should the more performant approach. And it already works without enhancing built-in shaders.

I agree it would be more interesting to have texture offsets per instance. But instead of using the code-injection pattern, I would favor a setUVOffsetAt() method in InstancedMesh.

@WestLangley
Copy link
Collaborator Author

instead of using the code-injection pattern

If we are going to avoid recommending the use of code-injection via onBeforeCompile(), we should remove the example, because the example uses it.

Your suggestion is a work-around to control instance visibility, but it is not a replacement for the example, IMHO.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 10, 2022

I've revisited this PR since I have seen a community member using the example's code for having colors per instance. The dev was not aware about setColorAt(). Maybe is better to remove the example to avoid such confusion.

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.

2 participants