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

Add and update icons and Update icon automation (debug mode) #14942

Merged
merged 7 commits into from
Feb 22, 2024

Conversation

zeusongit
Copy link
Contributor

@zeusongit zeusongit commented Feb 15, 2024

Purpose

The PR adds and updates node icons shared internally.
Total 600 icons are updated and 312 icons are added.

The size difference between old and new icons is ~9MB as we are using 400dpi icons now.

Before:
Screenshot 2024-02-14 at 7 09 49 PM

After:
Screenshot 2024-02-14 at 6 58 53 PM

Before:
Screenshot 2024-02-14 at 7 10 40 PM

After:
Screenshot 2024-02-14 at 7 10 43 PM

Better resolution:
Picture1

The new automation is under DebugMode : UpdateNodeIcons
It requires the path to new icons and will do everything else, given that:

  • The icon names are in the correct format: Named after Node's FullName or IconName if it has overloads, and suffixed by Large or Small (png)

There is a review step that will scan the directory for icons, and load them with old icons for easy comparison, logs are generated and stored as well for troubleshooting.
The second step is to update icons after the result from first review step seems satisfactory.
A backup file is also generated for the updated resx files, just in case.

DynamoSandbox_hY8x82Ws7N

DynamoSandbox_Tm0GaWUX5x

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
  • This PR contains no files larger than 50 MB

Release Notes

  • Icon Refresh

Reviewers

@DynamoDS/dynamo

@zeusongit zeusongit requested a review from QilongTang February 15, 2024 00:21
Copy link

github-actions bot commented Feb 15, 2024

UI Smoke Tests

Test: success. 2 passed, 0 failed.
TestComplete Test Result
Workflow Run: UI Smoke Tests
Check: UI Smoke Tests - net8.0

@QilongTang QilongTang added this to the 3.1 milestone Feb 15, 2024
@QilongTang
Copy link
Contributor

Nice automation, just curious did you use the 100 subfolder or the 400 subfolder?

@zeusongit
Copy link
Contributor Author

Nice automation, just curious did you use the 100 subfolder or the 400 subfolder?

400 for all

@QilongTang
Copy link
Contributor

Nice automation, just curious did you use the 100 subfolder or the 400 subfolder?

400 for all

@Jingyi-Wen I think we were talking about integrating 100 ones but curious if 400 ones would bring any benefit? e.g. would resolution be better when zoomed in? @zeusongit If you dont mind, would you make a comparison using one of the node I updated previously v.s. the one updated by you, because I used 100 ones

@zeusongit
Copy link
Contributor Author

Nice automation, just curious did you use the 100 subfolder or the 400 subfolder?

400 for all

@Jingyi-Wen I think we were talking about integrating 100 ones but curious if 400 ones would bring any benefit? e.g. would resolution be better when zoomed in? @zeusongit If you dont mind, would you make a comparison using one of the node I updated previously v.s. the one updated by you, because I used 100 ones

In comparison, the 400 icons do look sharper than the 100 ones.
(The top ones are old and bottom is new)
cc

@@ -55,6 +55,7 @@ private static void RegisterDebugModes()
// Register app wide new debug modes here.
AddDebugMode("DumpByteCode", "Dumps bytecode to a log file in a folder called ByteCodeLogs located in the current working dirrectory.", false);
AddDebugMode("CrashOnNewNodeModel", "Crash when creating a new NodeModel. Works only on Debug builds", false);
AddDebugMode("UpdateNodeIcons", "This debug mode enables an automation that can be used to update node icons. https://wiki.autodesk.com/x/OACkf", true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need this? Regular builds do not have debug menu enabled right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started with this being hidden behind the debug mode, removed now

@QilongTang QilongTang merged commit 1c1d3e2 into DynamoDS:master Feb 22, 2024
20 of 22 checks passed
BitmapImage bi = new BitmapImage();

bi.BeginInit();
bi.StreamSource = new MemoryStream(System.Convert.FromBase64String(s));
Copy link
Member

Choose a reason for hiding this comment

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

@zeusongit I think you need to carefully examine if this code leaks. You never dispose this stream.

namespace Dynamo.Wpf.Views.Debug
{
/// <summary>
/// Interaction logic for DebugModesWindow.xaml
Copy link
Member

Choose a reason for hiding this comment

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

comment incorrect.

@mjkkirschner
Copy link
Member

mjkkirschner commented Feb 23, 2024

@zeusongit this PR brings in lots of new public methods that need to be added to the unshipped.txt file.
https://github.com/DynamoDS/Dynamo/blob/master/src/DynamoCoreWpf/PublicAPI.Unshipped.txt

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.

4 participants