Switch off 3D rendering on out of memory #10535

mmisol · 2020-04-02T17:47:59Z

Purpose

Switch off 3D rendering on out of memory

When rendering too much geometry, an OutOfMemoryException can be
raised by Helix, which causes Dynamo to crash.

This addresses the problem by reacting to the error. The background
geometry preview is disabled and a message dialog is shown to the user
communicating the problem.

Screenshot of the message:

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.

Reviewers

@mjkkirschner

FYIs

@QilongTang @aparajit-pratap

mjkkirschner · 2020-04-02T18:33:38Z

src/DynamoCore/Models/DynamoModel.cs

+            }
+        }
+
+        public void Report3DPreviewOutage(string summary, string description)


does this need to be public?

aparajit-pratap · 2020-04-03T18:47:51Z

src/DynamoCoreWpf/Properties/Resources.en-US.resx

@@ -2239,4 +2239,10 @@ Uninstall the following packages: {0}?</value>
  <data name="NodeTooltipOriginalName" xml:space="preserve">
    <value>Original node name: </value>
  </data>
+  <data name="RenderingMemoryOutageDescription" xml:space="preserve">
+    <value>Please check if the amount of generated geometry is correct. If it is, it might be necessary to disable preview for some nodes before reactivating the background 3D preview.</value>


How about wording this as The amount of generated geometry might be too large. If it is necessary to create them, it might be necessary to disable .... or consider generating fewer geometry objects.?

Isn't it kind of a contradiction to ask to consider generating fewer geometry objects after saying If it is necessary to create them? Also this message does not really mention the hint of switching off preview for some nodes, which should be useful.

I meant to leave the part about disabling preview from your original message, which is what the ... (ellipses) indicates. I just thought that instead of geometry is not "correct" you should mention that the amount of geometry created is large.

Oh, I see. The message actually says amount of generated geometry is correct. I meant to say that the amount may be large by accident. For instance, the user could have inadvertently enabled the repeat parameter of List.Combinations when it was not needed.

mjkkirschner · 2020-04-03T22:15:11Z

src/DynamoCoreWpf/ViewModels/Watch3D/HelixWatch3DViewModel.cs

@@ -219,7 +221,7 @@ protected override void OnActiveStateChanged()
        }

        public event Action<Model3D> RequestAttachToScene;
-        protected void OnRequestAttachToScene(Model3D model3D)
+        protected virtual void OnRequestAttachToScene(Model3D model3D)


unfortunately this method will likely be removed / unused in the helix-upgrade branch - it might be possible to retain it and redirect it to do something useful with the new helix api though.

Will this affect us when we cherry pick this test into helix-upgrade now?

To build this as a unit test I just needed an overridable method called from AggregateRenderPackages. I guess I could use another method with these characteristics in helix-upgrade if it exists, or I could add this one with no behaviour if not. Does that sound ok?

mmisol · 2020-04-06T15:03:14Z

Hi @Amoursol . Could I ask you to take a look at the dialog shown in the attached screenshot and give your opinion about it? It's related to Dynamo switching of background 3D preview when the amount of geometry is too large. Thanks in advance :)

Amoursol · 2020-04-06T15:16:43Z

Hello @mmisol - A couple of quick questions / clarifications.

Will all geometry fail to render if there is too much? Or will it render a subset?
Do we have any known thresholds on when rendering fails?
If the user turns of Geometry Preview on some nodes, will the background preview automatically showcase the remaining geometry if it can? Or do you need to re-run?
Is there currently a way to track the 'fullness' of memory? So users can see if they are approaching this threshold?

mmisol · 2020-04-06T15:45:23Z

Hi @Amoursol . Those are very relevant questions, here are some answers:

The rendering engine actually suffers an internal failure, so disabling the background 3D preview is a protection measure from further problems. Here is a screenshot of the kind of problems we can get (this shows up in the background preview if not disabled)

This depends on the internals of Helix. We could probably find the answer there, but it would take some effort. That's not the scope here but, is it something we may consider doing @mjkkirschner ?
I'm not sure. Nothing I have done here changes the current behaviour of switching on/off the 3d preview. The geometry objects are generated and should still be there, so probably not.
This is kind of related to 2. It would requires us to know the internal structures of Helix to pre-determine how much memory the rendering would take.

Amoursol · 2020-04-06T16:18:37Z

@mmisol - OK cool, thanks for that! We won't look to creep scope - just wanted to know how to message appropriately 😄

To dive deeper into #1:
A) When the engine suffers an internal failure, does that break the existing rendering? Or only further rendering? i.e if I have 1000 balls, and it breaks at ball 750, do I still see 749 balls? Or do they all disappear?
B) When the engine breaks, do they need to restart Dynamo? Or will a new run resolve it?

mmisol · 2020-04-06T16:59:02Z

@Amoursol Sounds good :) About the questions:

A) Yes, the current rendering breaks. It's a two-step process where we remove and then add render packages. In the case I tested, what the user sees is that the curves disappear and eventually the error occurs preventing further rendering.
B) From what I could see the memory usage goes down to normal levels after switching off rendering, so they should be able to continue using Dynamo.

Amoursol · 2020-04-06T18:55:00Z

@mmisol If I'm reading this correctly then the error message above doesn't tell the user why their existing rendering has disappeared?

Might be more appropriate to include something like: "Dynamo has ran out of memory trying to render your Geometry causing it to disappear. Please check if you intended to render this amount of Geometry and if you did, then consider turning off the preview of other nodes within your graph or lowering the amount of Geometry you wish to render. Dynamo will attempt to turn back on the 3D Background Preview and render your geometries when you next switch back from graph view navigation."

Now that's a bit of a mouthful so please do feel free to reword, but it's also a bit more expressive of what's happening enabling the user to understand where they can make changes to help and why it's failing in the first place.

mmisol · 2020-04-06T19:03:58Z

@Amoursol You are right, it doesn't say why it disappeared, that's a good point. I think your message is great, I will copy it as is. Thank you!

mjkkirschner · 2020-04-06T20:37:41Z

How about?
"Dynamo has run out of memory trying to render your geometry. The geometry preview has been disabled.

Please check if you intended to render this amount of geometry, and consider turning off the preview of other nodes within your graph, lowering the amount of Geometry you wish to render, or turning down the render percision.

Dynamo will attempt to turn the 3D Background Preview back on, and render the geometry when you next switch back from graph view navigation."

Also is this last sentence actually true?

aparajit-pratap · 2020-04-06T20:42:27Z

Dynamo will attempt to turn the 3D Background Preview back on, and render the geometry when you next switch back from graph view navigation.

I had the same question about this statement. Is it true that the background preview will turn back on on its own? I think the user might have to turn it back on.

mmisol · 2020-04-06T21:00:31Z

You guys are right. When the background 3D preview is disabled, the user cannot switch between graph and 3D preview anymore. Removed that last part to avoid confusion.

Let me know if it looks good now.

Amoursol · 2020-04-06T21:17:45Z

It was more of a query hence the "If I understand correctly" 😊 Your version LGTM @mjkkirschner and I'm happy either way, was just around clarity on how the user can avoid the problem or remedy it that I was concerned with.

WIP

8708787

mmisol requested a review from mjkkirschner April 2, 2020 17:48

mmisol added the DNM Do not merge. For PRs. label Apr 2, 2020

mjkkirschner reviewed Apr 2, 2020

View reviewed changes

Create test and make method internal

e6730b2

mmisol changed the title ~~[WIP] Switch off 3D rendering on out of memory~~ Switch off 3D rendering on out of memory Apr 3, 2020

mmisol removed the DNM Do not merge. For PRs. label Apr 3, 2020

aparajit-pratap reviewed Apr 3, 2020

View reviewed changes

mjkkirschner reviewed Apr 3, 2020

View reviewed changes

Merge remote-tracking branch 'origin' into render_out_of_memory

83bae96

Change message shown to user

64a87ef

Change message slightly and remove last part

8bc14a1

mjkkirschner approved these changes Apr 7, 2020

View reviewed changes

mmisol merged commit 3be3302 into DynamoDS:master Apr 7, 2020

mmisol mentioned this pull request Apr 14, 2020

Handle memory outage on render (cherry pick) #10569

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch off 3D rendering on out of memory #10535

Switch off 3D rendering on out of memory #10535

mmisol commented Apr 2, 2020 •

edited

Loading

mjkkirschner Apr 2, 2020

aparajit-pratap Apr 3, 2020

mmisol Apr 6, 2020

aparajit-pratap Apr 6, 2020

mmisol Apr 6, 2020

mjkkirschner Apr 3, 2020

mmisol Apr 6, 2020

mmisol commented Apr 6, 2020 •

edited

Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020 •

edited

Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020 •

edited

Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020

mjkkirschner commented Apr 6, 2020

aparajit-pratap commented Apr 6, 2020

mmisol commented Apr 6, 2020

Amoursol commented Apr 6, 2020

Switch off 3D rendering on out of memory #10535

Switch off 3D rendering on out of memory #10535

Conversation

mmisol commented Apr 2, 2020 • edited Loading

Purpose

Declarations

Reviewers

FYIs

mjkkirschner Apr 2, 2020

Choose a reason for hiding this comment

aparajit-pratap Apr 3, 2020

Choose a reason for hiding this comment

mmisol Apr 6, 2020

Choose a reason for hiding this comment

aparajit-pratap Apr 6, 2020

Choose a reason for hiding this comment

mmisol Apr 6, 2020

Choose a reason for hiding this comment

mjkkirschner Apr 3, 2020

Choose a reason for hiding this comment

mmisol Apr 6, 2020

Choose a reason for hiding this comment

mmisol commented Apr 6, 2020 • edited Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020 • edited Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020 • edited Loading

Amoursol commented Apr 6, 2020

mmisol commented Apr 6, 2020

mjkkirschner commented Apr 6, 2020

aparajit-pratap commented Apr 6, 2020

mmisol commented Apr 6, 2020

Amoursol commented Apr 6, 2020

mmisol commented Apr 2, 2020 •

edited

Loading

mmisol commented Apr 6, 2020 •

edited

Loading

mmisol commented Apr 6, 2020 •

edited

Loading

mmisol commented Apr 6, 2020 •

edited

Loading