8000 Remove the unnecessary fields in DpiHelper by lindexi · Pull Request #6851 · dotnet/wpf · GitHub
[go: up one dir, main page]
Skip to content

Remove the unnecessary fields in DpiHelper #6851

New issue 8000

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 13, 2025
Merged

Remove the unnecessary fields in DpiHelper #6851

merged 1 commit into from
May 13, 2025

Conversation

lindexi
Copy link
Member
@lindexi lindexi commented Jul 27, 2022 •
edited by ghost
Loading

Description

We can merge the _transformToDevice and _transformToDip fields to local variable. Why we define the _transformToDevice and _transformToDip fields? Because we use the two fields to store the DPI in the previous version, and the code is:

        private static Matrix _transformToDevice;
        private static Matrix _transformToDip;

        [SuppressMessage("Microsoft.Performance", "CA1810:InitializeReferenceTypeStaticFieldsInline")]
        static DpiHelper()
        {
            using (SafeDC desktop = SafeDC.GetDesktop())
            {
                int deviceCaps1 = NativeMethods.GetDeviceCaps(desktop, DeviceCap.LOGPIXELSX);
                int deviceCaps2 = NativeMethods.GetDeviceCaps(desktop, DeviceCap.LOGPIXELSY);
                DpiHelper._transformToDip = Matrix.Identity;
                DpiHelper._transformToDip.Scale(96.0 / (double)deviceCaps1, 96.0 / (double)deviceCaps2);
                DpiHelper._transformToDevice = Matrix.Identity;
                DpiHelper._transformToDevice.Scale((double)deviceCaps1 / 96.0, (double)deviceCaps2 / 96.0);
            }
        }

In order to support Per-Moniter v2, we will use the DPI in per window and we do not get the dpi from desktop once. It is means that we do not need these two fields anymore.

Customer Impact

None.

Regression

None.

Testing

Just CI.

Risk

Low. Unless some developers rely on using reflection to get fields.

Microsoft Reviewers: Open in CodeFlow

@lindexi lindexi requested a review from a team as a code owner July 27, 2022 00:55
@ghost ghost assigned lindexi Jul 27, 2022
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jul 27, 2022
@ghost ghost requested review from dipeshmsft, singhashish-wpf and SamBent July 27, 2022 00:55
@ghost ghost added the Community Contribution A label for all community Contributions label Jul 27, 2022
@lindexi lindexi mentioned this pull request Jul 27, 2022
Open
@pchaurasia14
Copy link
Member

/azp run

@azure-pipelines
Copy link
Azure Pipelines successfully started running 1 pipeline(s).

@dipeshmsft
Copy link
Member

/azp run

@azure-pipelines
Copy link
No pipelines are associated with this pull request.

@dipeshmsft
Copy link
Member

/azp run

@azure-pipelines
Copy link
Azure Pipelines successfully started running 1 pipeline(s).

@dipeshmsft dipeshmsft self-assigned this Oct 10, 2023
h3xds1nz
h3xds1nz reviewed Jan 28, 2025
View reviewed changes
Copy link
Member
@h3xds1nz h3xds1nz left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM, great find @lindexi

@himgoyalmicro
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link
Azure Pipelines successfully started running 1 pipeline(s).

himgoyalmicro
himgoyalmicro previously approved these changes Mar 24, 2025
View reviewed changes
@himgoyalmicro
Copy link
Contributor

@lindexi This PR is good to be merge can you please resolve the conversations?

@codecov Codecov
Copy link
codecov bot commented Mar 24, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@c8742b5). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...tionFramework/System/Windows/Standard/DpiHelper.cs 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #6851   +/-   ##
=======================================
  Coverage        ?   13.48%           
=======================================
  Files           ?     3316           
  Lines           ?   664822           
  Branches        ?    74651           
=======================================
  Hits            ?    89637           
  Misses          ?   572582           
  Partials        ?     2603
Flag Coverage Δ
Debug 13.48% <0.00%> (?)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lindexi lindexi force-pushed the t/lindexi/DpiHelper branch from f763970 to 06f7f4f Compare May 13, 2025 06:26
@himgoyalmicro himgoyalmicro merged commit c572f8e into dotnet:main May 13, 2025
8 checks passed
@himgoyalmicro
Copy link
Contributor

Thank you @lindexi, for your consistent and valuable contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@h3xds1nz h3xds1nz h3xds1nz left review comments

@himgoyalmicro himgoyalmicro himgoyalmicro approved these changes

@dipeshmsft dipeshmsft Awaiting requested review from dipeshmsft

@singhashish-wpf singhashish-wpf Awaiting requested review from singhashish-wpf

@SamBent SamBent Awaiting requested review from SamBent

Assignees

@lindexi lindexi

@dipeshmsft dipeshmsft

Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lindexi @pchaurasia14 @dipeshmsft @himgoyalmicro @h3xds1nz
0