DYN-3896 Add support for geometry instancing in render pipeline by saintentropy · Pull Request #11914 · DynamoDS/Dynamo

saintentropy · 2021-08-05T03:05:14Z

Purpose

https://jira.autodesk.com/browse/DYN-3896

The purpose of this PR is to add geometry instancing support in the render pipeline for the 3D background preview. This work build on the PR where we modified the render pipeline to gather one RenderPackage per node. Instancing can be utilized to reduce tessellation overhead for symmetrical geometry types (cube, rectangle, sphere, circle, ellipse, cylinders, coordinate systems, and planes) as well as dramatically improve render time and memory usage required for the 3D Preview. Perf and memory testing will depend on the object type and size of tessellation data but an example of 30,000 spheres reduces the tessellation and render time from 23 to 2 sec and memory size of Dynamo from 5gig to 500 mb.

(500,000 Spheres)

Specifically this PR adds to Dynamo:

Adds IInstanceableItem interface that follows the pattern of IGraphicItem where the interface is applied to object who can be rendered via instancing. The interface add a method to the object to define a base set of tessellation data and methods to set transformation matrices for any instance geometry. The transforms handle the transition, rotation, and scaling of the base tessellation data to the objects placement in 3D space
Adds IRenderInstances interface for extending RenderPackges to support storing base tessellation data and instance mtarices
Adds support the tessellation pass to gather instancing data if the object supports it before falling back to standard tessellation.
Adds support the HelixWatch3DViewModel to utilize the Helix instancing APIs to render geometry with associated instance matrices.

There will be an associated PR for LibG to utilize these new interfaces and new API's

Todo

discuss adding instance color setting optionally
update tests

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

Reviewers

@aparajit-pratap @mjkkirschner

FYIs

@QilongTang @jasonstratton

mjkkirschner

I will need to continue reviewing this and the LibG changes - at the moment I don't yet understand the changes in the HelixWatch3dViewModel

src/NodeServices/GraphicsDataInterfaces.cs

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs

mjkkirschner · 2021-11-01T15:27:28Z

@saintentropy ~~do you intend to add tests to cover your new functionality? For example a class that actually implements the new interfaces so this can be tested without LibG?~~

discussed with @saintentropy - I will work to add some tests for this repo for this functionality.

add viewmodel test that renders instances add test classes add test file

src/DynamoCoreWpf/Rendering/HelixRenderPackage.cs

src/DynamoCore/Scheduler/UpdateRenderPackageAsyncTask.cs

src/DynamoCoreWpf/Rendering/HelixRenderPackage.cs

src/NodeServices/GraphicsDataInterfaces.cs

aparajit-pratap · 2021-11-12T15:51:40Z

src/NodeServices/GraphicsDataInterfaces.cs

+        /// <summary>
+        /// A flag used to indicate if the current geometrical configuration of an item has instance information.
+        /// </summary>
+        bool InstanceInfoAvailable { get; }


If something does not have instance information, it won't be an IInstanceableItem, right?

aparajit-pratap · 2021-11-12T15:53:22Z

src/NodeServices/GraphicsDataInterfaces.cs

+        /// <param name="package">The render package, where graphics data to be
+        /// pushed.</param>
+        /// <param name="parameters"></param>
+        void AddBaseTessellation(IRenderPackage package, TessellationParameters parameters);


Same q here.

src/NodeServices/GraphicsDataInterfaces.cs

…ancing

try making interfaces internal

…into instancing

small fixes internals visible for test update tests

mjkkirschner · 2022-05-16T18:56:47Z

tests passed here:
https://master-5.jenkins.autodesk.com/job/Dynamo/job/DynamoSelfServe/job/pullRequestValidation/6190/

src/DynamoCore/Scheduler/UpdateRenderPackageAsyncTask.cs

saintentropy · 2022-05-16T23:45:17Z

src/DynamoCoreWpf/Rendering/HelixRenderPackage.cs

+        /// <returns></returns>
+        public bool ContainsTessellationId(Guid id)
+        {
+            return MeshVertexRangesAssociatedWithInstancing.ContainsKey(id) || LineVertexRangesAssociatedWithInstancing.ContainsKey(id);


@mjkkirschner Hmmmmm.... Not sure but we may need to handle a transform that has both mesh and line type tessellation data. Maybe when edges are on. I can't remember how that case was supported.

It might be a bit of a hack but the LIbG portion could actually handle this ^^^ I think. I always wondered why we didn't handle the ShowEdges in the base tessellation call. Maybe puts too much ownership on those implementing IGraphicItem. In this case we are still internal so it may work to let the instanceableItem handle the data. In the case of a cube, AddBaseTessellation(instancingPackage, factory.TessellationParameters) could inspect the TessellationParameters and add only a mesh or a mesh and line data if edges are needed. They would have to have separate GUIDs. The subsequent call to AddInstance(instancingPackage, factory.TessellationParameters, labelKey) could also add the correct instance references fo either mesh only or mesh and line data.

This may also be follow up task?

⚠️ I'll take a quick look but ,if nothing obvious and simple I think I'll have to file a followup as the allotted time for this task is running out.

I wonder if we can just push the line data to the baseTessellation from this method:

Dynamo/src/DynamoCore/Scheduler/UpdateRenderPackageAsyncTask.cs

Line 262 in 9740b24

if (factory.TessellationParameters.ShowEdges)

or always generate the edge data in baseTessellation but only using it (extracting it from the render package) if show edges is on.

For the current state of the PR, I think we need to decide the following:

When show_edges is on, we don't use instancing OR -

When show_edges is on, we use instancing but don't draw edges... OR

When show_edges is on, we use instancing for the meshes, but push a bunch of non instanced edge data?

Could be also...
4. Edges added within instancing implementation.

saintentropy · 2022-05-17T00:16:03Z

test/Engine/FFITarget/InstanceableClass.cs

+    }
+
+    /// <summary>
+    /// A test class that creates a pyramid using instancing.


add test that will fail until libG changes are merged

mjkkirschner · 2022-05-17T18:32:19Z

src/VisualizationTests/HelixWatch3DViewModelTests.cs


        }
+        [Test]
+        [Category("Failure")]


we can enable this after the LibG changes are merged, it will fail until then because the number of instances will be incorrect.

saintentropy · 2022-05-18T14:59:10Z

Hey @mjkkirschner, the show edges implementation looks good. This PR LGTM

saintentropy added 10 commits July 19, 2021 12:13

Add instancing support to DynamoMesh sharder

987381f

Add instancing interfaces to GraphicsDataInterfaces for instance support

0824814

Add IRenderInstances implementation to HelixRenderPackage

37ceb00

Add support to HelixWatch3DViewModel to add instance data when pressent

3aecdc4

Handle adding instance data when object is IInstanceableItem

3276e67

Update interface to add tessellation params

ca899f7

add line instancing to render package

0de95b0

Support rendering of line instances

f901cb6

add todo for labels

774d66e

Merge branch 'master' into Instancing

c69a6cc

saintentropy added the WIP label Aug 5, 2021

saintentropy added 3 commits August 18, 2021 22:38

Add bool to control instance availablity

c379ed4

Merge branch 'master' into Instancing

c2f34d2

Add parameter to support labeling within instance set

6a08c8a

saintentropy changed the title ~~WIP DYN-3896 Add support for geometry instancing in render pipeline~~ DYN-3896 Add support for geometry instancing in render pipeline Aug 19, 2021

saintentropy removed the WIP label Aug 23, 2021

Merge branch 'master' into Instancing

a95a881

saintentropy added the performance label Oct 18, 2021

mjkkirschner self-assigned this Oct 28, 2021

mjkkirschner requested changes Oct 28, 2021

View reviewed changes

saintentropy and others added 4 commits November 4, 2021 19:46

Merge branch 'master' into Instancing

628392b

add simple renderpackage tests

0a5b8f3

add viewmodel test that renders instances add test classes add test file

add instancing image comparison test 1

a0302cd

add image comparison test for instance class with no data

e7170fe

aparajit-pratap reviewed Nov 12, 2021

View reviewed changes

mjkkirschner added 4 commits May 11, 2022 12:47

Merge branch 'master' of https://github.com/DynamoDS/Dynamo into inst…

f60c9b6

…ancing

review comments to HelixWatch3dViewModel and GraphicsDataInterface files

f407a71

builds and moved to value tuple

60aabb8

it builds

a8caa7b

mjkkirschner added 9 commits May 12, 2022 14:13

fix some comments

c05c323

try making interfaces internal

Merge branch 'instancetests' of https://github.com/mjkkirschner/Dynamo …

0da5120

…into instancing

initialize internal collection properties

cfb521c

small fixes internals visible for test update tests

new test for transformable and instancing

fec13a9

control instancing with feature flag

e38a40b

update

af38da1

test mode should enable instancing flag

1850bc0

update shaders

df92121

fix test, add 1 more

2c9551f

saintentropy commented May 16, 2022

View reviewed changes

src/DynamoCore/Scheduler/UpdateRenderPackageAsyncTask.cs Outdated Show resolved Hide resolved

saintentropy commented May 16, 2022

View reviewed changes

saintentropy commented May 17, 2022

View reviewed changes

handle show edges

52331e1

add test that will fail until libG changes are merged

mjkkirschner reviewed May 17, 2022

View reviewed changes

TODO

b4b2fe7

mjkkirschner merged commit 581b2ef into DynamoDS:master May 18, 2022

Conversation

saintentropy commented Aug 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Reviewers

FYIs

Uh oh!

mjkkirschner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjkkirschner commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aparajit-pratap Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

aparajit-pratap Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mjkkirschner commented May 16, 2022

Uh oh!

Uh oh!

saintentropy May 16, 2022

Choose a reason for hiding this comment

Uh oh!

saintentropy May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saintentropy May 17, 2022

Choose a reason for hiding this comment

Uh oh!

mjkkirschner May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saintentropy May 17, 2022

Choose a reason for hiding this comment

Uh oh!

saintentropy May 17, 2022

Choose a reason for hiding this comment

Uh oh!

mjkkirschner May 17, 2022

Choose a reason for hiding this comment

Uh oh!

saintentropy commented May 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

saintentropy commented Aug 5, 2021 •

edited

Loading

mjkkirschner commented Nov 1, 2021 •

edited

Loading

saintentropy May 17, 2022 •

edited

Loading

mjkkirschner May 17, 2022 •

edited

Loading