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

ASM Scaling UI #7455

Merged
merged 7 commits into from
Jan 9, 2017
Merged

ASM Scaling UI #7455

merged 7 commits into from
Jan 9, 2017

Conversation

aparajit-pratap
Copy link
Contributor

Purpose

This is a merge of #6991 with the addition of some minor fixes to force re-execute the graph only when the geometry scale setting is changed and not every time the dialog is opened.

Closing #6991 ...

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. - Refer ASM Scaling UI #6991
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

@aparajit-pratap aparajit-pratap mentioned this pull request Dec 26, 2016
3 tasks
@aparajit-pratap aparajit-pratap merged commit 032e9ca into DynamoDS:master Jan 9, 2017
@aparajit-pratap aparajit-pratap deleted the Homework branch January 9, 2017 06:27
@@ -49,6 +49,7 @@ public WorkspaceInfo()
double cx = 0;
double cy = 0;
double zoom = 1.0;
double scaleFactor = 1.0;
Copy link
Member

Choose a reason for hiding this comment

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

@ramramps @QilongTang @sm6srw seems we may need to serialize this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

In any case, this gets stored in DYN and Flood needs this?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have zoom property serialized to workspace db, currently I believe we have no case utilizing the scaleFactor, but if we want to have this ASM scaling in Flood editor then we will probably need this

Copy link
Member

@mjkkirschner mjkkirschner Jan 9, 2017

Choose a reason for hiding this comment

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

there is a UI associated with this which webUI might need - in addition I am not sure if we need to scale the tessellated geometry after it is returned from ASM or if LibG is doing this scaling of the tessellated geometry @aparajit-pratap can you fill us in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mjkkirschner yes, the scale factor needs to be serialized to the DYN file. This PR takes care of that for desktop. Yes, you would need to introduce the same in Flood as well as the new UI. The tessellated geometry is scaled back after it is returned from ASM and this is taken care of in LibG. Let me know if you have further questions.

Copy link
Member

Choose a reason for hiding this comment

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

thanks @aparajit-pratap - good to know, first question that comes to mind which is not obvious to me from this code is where does the scalefactor get injected into LibG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scale factor is passed into LibG using WorkspaceEvents in DynamoServices. These are the related PR's to enable this:
#6967
#7391

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.

5 participants