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

[Test] Experiment with using Native Containers in query API #411

Open
wants to merge 3 commits into
base: refactor/remove-gpu-readback
Choose a base branch
from

Conversation

moosichu
Copy link
Contributor

Seeing what the impact of this would be, if we were to actually support these we would probably have to keep supporting the old API a while as well which would require effort. Another issue with this is the fact that NativeArray won't support ref returns until Unity updates the version of C# it uses, which will remove a lot of the annoying assining-out to a variable and assigning back again.

But this does seem to work! If we were to have jobified queries as well, it would probably be best to have it work with an API like this.

@NuclearCookie
Copy link

I'm querying the ocean height around 90 times per frame, using the ShapeGerstnerBatched script.
This took 2.5ms on a desktop PC.
I have also converted what I needed to use NativeArrays, unity mathematics and made a job for ComputeDisplacements, and it the cost dropped to 1ms.
Looks well worth the effort.

@moosichu
Copy link
Contributor Author

Thank you for the feedback! I'm personally keen to move over to Unity.Mathematics and NativeContainers by default for most of our API, but mainting backwards compatibility is important so it's something we will have to think carefully about, especialy as it introduces dependencies on Unity packages and we already have to support 3 versions of crest across many more versions of Unity already.

When the asset store supports it, we plan on making all versions of Crest unity packages which will allow us to better able declare dependencies appopriately as well.

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