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

Create shared CsWin32 with System.Drawing.Common #10525

Merged
merged 5 commits into from
Jan 4, 2024

Conversation

JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Dec 22, 2023

This is a rough prototype of a shared interop with System.Drawing.Common. It introduces a new base assembly for shared CsWin32 and other low-level support code.

Converted HGLOBAL, DEVMODE and related stuff to the new shared pattern. Also used the CsWin32 generated GdiplusStartup.

I have almost no idea if packaging is correct here. Assuming it isn't.

crefs don't work right when there is overlap in internals. #10524 tracks this, and this change disables the relevant warnings.

I've only smoke tested this with the WinFormsControlsTest.

Microsoft Reviewers: Open in CodeFlow

This is a rough prototype of a shared interop with System.Drawing.Common. It introduces a new base assembly for shared CsWin32 and other low-level support code.

Converted HGLOBAL, DEVMODE and related stuff to the new shared pattern. Also used the CsWin32 generated GdiplusStartup.

I have almost no idea if packaging is correct here. Assuming it isn't.

`crefs` don't work right when there is overlap in internals. dotnet#10524 tracks this, and this change disables the relevant warnings.

I've only smoke tested this with the WinFormsControlsTest.
@JeremyKuhne JeremyKuhne added the draft draft PR label Dec 22, 2023
@JeremyKuhne JeremyKuhne requested a review from a team as a code owner December 22, 2023 22:14
@ghost ghost assigned JeremyKuhne Dec 22, 2023
@JeremyKuhne JeremyKuhne marked this pull request as draft December 22, 2023 22:14
@JeremyKuhne
Copy link
Member Author

Note that I'm also hitting some weird file locking during builds.

This is not necessarily the way we'll go. There are definitely benefits in simpler sharing of low-level code, but we still need to discuss options.

Mostly out for the holidays so unlikely to respond to anything until I'm back.

XML cref tags don't resolve the same way code does. Until we can get this resolved we need to turn this off in
order to use CsWin32 in both System.Drawing and System.Windows.Forms.
-->
<NoWarn>$(NoWarn);CS1574;CS1580</NoWarn>
Copy link
Member

Choose a reason for hiding this comment

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

❓ Are you able to work around this issue using the approach described here?
#10320 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't specify the root namespace for CsWin32 currently.

Copy link
Member

Choose a reason for hiding this comment

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

I believe you may have misunderstood my comment. My proposed solution is only relevant for cases where you cannot specify the root namespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

My proposed solution is only relevant for cases where you cannot specify the root namespace.

@sharwell that is every case afaik

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

👍

IntPtr modeHandle = _printerSettings.GetHdevmode();
IntPtr modePointer = Kernel32.GlobalLock(new HandleRef(this, modeHandle));
Gdi32.DEVMODE mode = Marshal.PtrToStructure<Gdi32.DEVMODE>(modePointer)!;
HGLOBAL modeHandle = (HGLOBAL)_printerSettings.GetHdevmode();
Copy link
Member

Choose a reason for hiding this comment

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

seems like we are freeing this a lot. May want to consider adding a scope in the future 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely intend to :)

@JeremyKuhne JeremyKuhne removed the draft draft PR label Jan 4, 2024
@JeremyKuhne JeremyKuhne marked this pull request as ready for review January 4, 2024 22:53
@JeremyKuhne JeremyKuhne merged commit 0932981 into dotnet:main Jan 4, 2024
9 checks passed
@ghost ghost added this to the 9.0 Preview1 milestone Jan 4, 2024
@JeremyKuhne JeremyKuhne changed the title Prototype shared CsWin32 with System.Drawing.Common Create shared CsWin32 with System.Drawing.Common Jan 4, 2024
@JeremyKuhne JeremyKuhne deleted the sharedinterop branch January 4, 2024 22:57
@AustinWise AustinWise mentioned this pull request Jan 5, 2024
public Form1()
{
InitializeComponent();

_textBox = new RichTextBox
Copy link
Member

Choose a reason for hiding this comment

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

@JeremyKuhne - please remove the content, we all use this project for random stuff, it's purpose was to stay empty for easy testing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, missed it

@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants