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

Remove DeviceContext HWND handling #47081

Merged
merged 2 commits into from
Feb 3, 2021
Merged

Remove DeviceContext HWND handling #47081

merged 2 commits into from
Feb 3, 2021

Conversation

hughbe
Copy link
Contributor

@hughbe hughbe commented Jan 16, 2021

DeviceContext has a bunch of overhead - allocations, interop calls etc.

However, in the case of Graphics.CopyFromScreen, which is the only user of the HWND features in DeviceContext, we can avoid all of that and remove a bunch of code from DeviceContext.

Just call IntPtr screenDC = GetDC(IntPtr.Zero) and ReleaseDC(screenDC)

/cc @JeremyKuhne

@ghost
Copy link

ghost commented Jan 16, 2021

Tagging subscribers to this area: @safern, @tannergooding
See info in area-owners.md if you want to be subscribed.

Issue Details

DeviceContext has a bunch of overhead - allocations, interop calls etc.

However, in the case of Graphics.CopyFromScreen, which is the only user of the HWND features in DeviceContext, we can avoid all of that and remove a bunch of code from DeviceContext.

Just call IntPtr screenDC = GetDC(IntPtr.Zero) and ReleaseDC(screenDC)

Author: hughbe
Assignees: -
Labels:

area-System.Drawing

Milestone: -

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Can you remove IDeviceContext from DeviceContext? I don't think there is a dependency on it...

Looks good otherwise. Going forward, it might be worth trying to kill DeviceContext completely as I have in Windows Forms and using ref struct "scopes" for code clarity / perf.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@ghost
Copy link

ghost commented Jan 30, 2021

Hello @safern!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@safern safern merged commit 7a5e37e into dotnet:master Feb 3, 2021
@hughbe hughbe deleted the hwnd branch February 3, 2021 08:41
@ghost ghost locked as resolved and limited conversation to collaborators Mar 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants