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

Tune Dynamo rendering precision #8929

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

vkrushelnitskyi
Copy link

@vkrushelnitskyi vkrushelnitskyi commented Jun 14, 2018

Purpose

This pull request containt changes related to perfomence issue recieved from Alias users.
Changes list:

  • Min value of "Render precision" changed to 8
  • Tolerance will change to 0 when "Render presion" is in range 8-12

Result of render 5000 spheres with parameter 8/0 about 5 seconds and with default parameters more than 10 minute.

Declarations

Check these if you believe they are true

  • The code base 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]

Reviewers

@aparajit-pratap

@mjkkirschner
Copy link
Member

@vkrushelnitskyi can you take images of the difference between these results?

@vkrushelnitskyi
Copy link
Author

8_0
default

@mjkkirschner
Copy link
Member

do you have some profiled results of the gains here for various inputs?

Also can you explain why the tolerance is turned down for precision values in that specific range and not lower?

@aparajit-pratap
Copy link
Contributor

@mjkkirschner a tolerance of zero basically disables a dynamic surface tolerance chosen by the ASM faceter. A tolerance of -1 enables it while any other positive value considers it as user input. @vkrushelnitskyi so why have you disabled dynamically chosen tolerance value in the case of lower faceting precision? Does this improve performance as well?

@mjkkirschner
Copy link
Member

@aparajit-pratap I am curious why it has been disabled for the range 8 - 12 only, and also I believe tolerance parameter is based on scale in certain modes - so I am looking for varied inputs/ times to make sure this does not introduce other performance issues with differently scaled geometry.

@vkrushelnitskyi
Copy link
Author

@aparajit-pratap Yes, this improve performance very much. For example render time of 5000 spheres with default value of "Render preciosion" and -1 value of tolarence take 1 minute 44 sec and 48 sec when tolerance set to 0.

@mjkkirschner because render results in not good with 0 tolerance, so 0 sets only in min range.

@tfinniga
Copy link

@vkrushelnitskyi I think @mjkkirschner 's question is about this block:

if (value >= 8 && value <= 12)
    factory.TessellationParameters.Tolerance = 0;

Why not just

if (value <= 12)
    factory.TessellationParameters.Tolerance = 0;

Why would you want to turn off tolerance only between 8 and 12? Why not have it off at 7 as well?

@vkrushelnitskyi
Copy link
Author

@tfinniga Oh, It is just a bad piece of code (value >=8), because min value for "Dynamo Precision" slider is 8. So, this value can not be 7 and less.

@mjkkirschner
Copy link
Member

@vkrushelnitskyi thanks! got it now. @alfarok you were right!

@mjkkirschner
Copy link
Member

This seems like a good idea to me given the performance improvement - does the default value of the slider change to 8 or only the minimum?

@aparajit-pratap thoughts?

@aparajit-pratap
Copy link
Contributor

@mjkkirschner @vkrushelnitskyi this is good to go. Thanks for the changes!

@vkrushelnitskyi
Copy link
Author

@mjkkirschner Only min.

@kronz kronz merged commit ec10f93 into DynamoDS:master Jun 20, 2018
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.

6 participants