Skip to content

Refactor icon initialization and make use of CreateIconResourceDirectory() for improved accuracy #13448

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 1 commit into
base: main
Choose a base branch
from

Conversation

DJm00n
Copy link
Contributor

@DJm00n DJm00n commented May 13, 2025

  • Refactored Icon.Initialize() method, replacing manual icon selection logic with LookupIconIdFromDirectoryEx() for better accuracy.
  • Introduced CreateIconResourceDirectory(), simplifying RT_GROUP_ICON data creation from ICO file data.
  • Removed redundant s_bitDepth field, relying on direct calculations when needed.
  • Eliminated unnecessary width/height calculations, replacing them with LR_DEFAULTSIZE flag in LookupIconIdFromDirectoryEx() call.
  • Optimized memory handling, avoiding unnecessary spans and copies while ensuring correct alignment.
  • Functionality remains unchanged, but initialization is now more efficient and maintainable
  • Three test cases where changed due to LookupIconIdFromDirectoryEx() icon selection logic (on par with used in Windows)

Customer Impact

  • LookupIconIdFromDirectoryEx() should fit icon selection logic used in LoadImage APIs in current and possible future Windows versions.

Regression?

  • No
Microsoft Reviewers: Open in CodeFlow

@DJm00n DJm00n requested a review from a team as a code owner May 13, 2025 08:42
@DJm00n DJm00n force-pushed the icon branch 4 times, most recently from 30a218b to e930ec2 Compare May 15, 2025 09:11
@DJm00n DJm00n changed the title Refactor icon initialization and make use of CreateIconResourceDirectory() for improved efficiency Refactor icon initialization and make use of CreateIconResourceDirectory() for improved accuracy May 15, 2025
…ory() for improved efficiency

- Refactored Icon.Initialize() method, replacing manual icon selection logic with LookupIconIdFromDirectoryEx() for better accuracy.
- Introduced CreateIconResourceDirectory(), simplifying RT_GROUP_ICON data creation from ICO file data.
- Removed redundant s_bitDepth field, relying on direct calculations when needed.
- Eliminated unnecessary width/height calculations, replacing them with direct system metric retrieval.
- Optimized memory handling, avoiding unnecessary spans and copies while ensuring correct alignment.
- Functionality remains unchanged, but initialization is now more efficient and maintainable
- Three test cases where changed due to LookupIconIdFromDirectoryEx() icon selection logic (used in Windows)
Copy link

codecov bot commented May 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.40438%. Comparing base (5821493) to head (c8c1ba3).
Report is 4 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                  @@
##                main      #13448          +/-   ##
====================================================
+ Coverage   62.21083%   97.40438%   +35.19355%     
====================================================
  Files           3213        1176        -2037     
  Lines         638302      352555      -285747     
  Branches       47201        5355       -41846     
====================================================
- Hits          397093      343404       -53689     
+ Misses        234153        8398      -225755     
+ Partials        7056         753        -6303     
Flag Coverage Δ
Debug 97.40438% <100.00000%> (+35.19355%) ⬆️
integration ?
production ?
test 97.40438% <100.00000%> (+0.00027%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant