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

[SPIKE] Use Helix instancing to save memory for large number of mesh geometry #11245

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Nov 13, 2020

Purpose

Trial implementation exploring using Helix instancing to render large number of mesh geometry.
This goes hand-in-hand with corresponding LibG changes here: https://git.autodesk.com/Dynamo/LibG/pull/1080

Assumptions

  • This assumes that any node producing mesh geometry (solids and surfaces) and participating in replication can potentially benefit from instancing while being rendered.

Issues

  • Instances do not currently display in the background preview for some reason
  • Geometry transforms are incorrect in LibG (e.g. if a Sphere is located away from the origin, it's transform matrix does not reflect its exact position)

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

}
else
{
instancingModel = new InstancingMeshGeometryModel3D();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm - we should inspect the EffectsManager at runtime to see if theres an effect for instancing meshes - it may try to use our shader.

{
baseId = baseId.Split(':')[0];
}
var id = ((p.RequiresPerVertexColoration || p.Colors != null || p.RequiresCustomTransform)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this somehow conflict with line this line below:
id = ((rp.RequiresCustomTransform) ? rp.Description : baseId) + PointsKey;

@mjkkirschner
Copy link
Member

mjkkirschner commented Nov 13, 2020

@aparajit-pratap I think in our vertex shaders we'll need to add something like this:

https://github.com/DynamoDS/Dynamo/blob/RC2.5.0_master/src/DynamoCoreWpf/ViewModels/Watch3D/Shaders/Custom.fx#L157

coming back this after thinking about it for another few minutes:
I think this is more likely to work:
https://github.com/helix-toolkit/helix-toolkit/blob/develop/Source/HelixToolkit.Native.ShaderBuilder/VS/vsMeshDefault.hlsl#L37

@mjkkirschner
Copy link
Member

Another thought on this type of approach - opposed to just replicated calls, it would also be good to try this on calls like move or scale, rotate etc- where instancing and transforming is exactly what the node does and instancing will definitely work.

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