Skip to content

[DYN-8894]-Performance improvements after hide proxy ports and group auto layout #16410

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ivaylo-matov
Copy link
Contributor

@ivaylo-matov ivaylo-matov commented Jul 18, 2025

Purpose

This PR is related to DYN-8894 and DYN-7845.

Changes in code revert performance degradation

Loading Manekin.dyn:

Declarations

Check these if you believe they are true

  • 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
  • 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
  • This PR contains no files larger than 50 MB
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

Changes in code revert performance degradation

Reviewers

@DynamoDS/eidos
@zeusongit
@reddyashish

FYIs

@dnenov
@achintyabhat

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8894

@zeusongit zeusongit requested review from a team and Copilot July 22, 2025 19:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements performance improvements for group annotations by adding controls to hide proxy ports and enable minimal size auto-layout. The changes restore performance levels after previous degradation, reducing loading time for Manekin.dyn from ~31 seconds back to ~23 seconds.

Key changes include:

  • Added new preference settings for controlling collapsed group port visibility and minimal size behavior
  • Enhanced group annotation view with toggle buttons for optional input ports and unconnected output ports
  • Implemented automatic layout adjustments when groups expand to prevent overlaps

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/settings/DynamoSettings-NewSettings.xml Added default settings for new port collapse preferences
test/DynamoCoreWpfTests/AnnotationViewModelTests.cs Updated test to handle additional undo step and use UnconnectedOutPorts collection
src/DynamoCoreWpf/Views/Menu/PreferencesView.xaml Added UI controls for collapsed group port preferences
src/DynamoCoreWpf/Views/Core/AnnotationView.xaml.cs Major refactoring to support port categorization and toggle functionality
src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs Added properties for new port collapse preferences
src/DynamoCoreWpf/ViewModels/Core/Converters/SerializationConverters.cs Added serialization for port toggle state properties
src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs Enhanced with port categorization, layout management, and preference handling
src/DynamoCoreWpf/UI/Converters.cs Enhanced color converter and added angle converter for toggle buttons
src/DynamoCore/Graph/Annotations/AnnotationModel.cs Added properties for port collapse states and layout management
src/DynamoCore/Configuration/PreferenceSettings.cs Added new preference properties for port collapse behavior
Files not reviewed (1)
  • src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)

src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs:147

  • The field name 'hasToggledOptionalInPorts' should follow PascalCase naming convention to be consistent with other boolean fields. It should be 'HasToggledOptionalInPorts'.
            }

src/DynamoCoreWpf/ViewModels/Core/AnnotationViewModel.cs:871

  • The word 'thoese' is misspelled and should be 'those'.
            // as we need those later for when we

@@ -475,7 +470,6 @@ private void GroupTextBlock_SizeChanged(object sender, SizeChangedEventArgs e)
if (ViewModel != null && (e.HeightChanged || e.WidthChanged) && !_isUpdatingLayout)
{
_isUpdatingLayout = true;

// Use Dispatcher.BeginInvoke to batch layout updates
Copy link
Preview

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment mentions 'batch layout updates' but the code only performs a single layout update operation. The comment should be more accurate about what the dispatcher call is achieving.

Suggested change
// Use Dispatcher.BeginInvoke to batch layout updates
// Use Dispatcher.BeginInvoke to schedule layout updates on the UI thread

Copilot uses AI. Check for mistakes.

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.

1 participant