Start using CsWin32 in System.Drawing.Common
#10320
Conversation
Fix Usings Replace ScreenDC with GetDcScope
ghost
assigned 
|
@JeremyKuhne I've linked directly to some files in |
|
Cc: @lonitra |
|
@elachlan can you please share the warnings you're getting? |
|
Here are the errors. I think the |
|
Maybe the common files such as |
| @@ -173,7 +173,7 @@ public static Font DefaultFont | |||
| Font? defaultFont = null; | |||
|
|
|||
| // For Arabic systems, always return Tahoma 8. | |||
| if ((ushort)Kernel32.GetSystemDefaultLCID() == 0x0001) | |||
| if ((ushort)PInvoke.GetSystemDefaultLCID() == 0x0001) | |||
There was a problem hiding this comment.
Perhaps add some constants for these hardcoded values? I see https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-lcid/63d3d639-7fd2-4afb-abbe-0d5b5551eef8 has a table with them.
There was a problem hiding this comment.
I imagine there is one from cswin32.
There was a problem hiding this comment.
there should be GetSystemDefaultLocaleName() call and compare with "ar" call instead. LCIDs are deprecated.
That was the plan. But I didn't want to move stuff quite yet. I was also wanting to work out folder structure in common if we move a lot of code files there. |
|
Related: |
|
@jaredpar we are currently running into lots of warnings along the lines of
The Primitives project is basically a container project for the Interop code. Just wondering how we can get unblocked here. |
|
@AArnott have you got any ideas? Can we even use CsWin32 in two different projects at the same time without issue? |
|
@elachlan Absolutely. CsWin32 can work on as many projects as you have. But Here is a sample that shows this. It has two projects, both wanting |
|
@JeremyKuhne Basically, the issue here is that the namespaces for cswin32 generated code are the same for both Only work around I see is aliasing the clashing types. Which isn't really scalable. Failing that, I think we would need to have a shared primitives project and move any |
This doesn't gel with my experimentation. That solution has two projects, both consume CsWin32 types that they generate. And an xml doc comment in the referencing project has a type reference to the CsWin32 type that they both generate, and there is no type resolution ambiguity warning emitted. |
|
I tried to reproduce the issue in the attached project, but haven't been successful so far. |
|
@jaredpar can you help connect me to the right person for this? It seems to be confined to however XML comments are resolving |
|
System.Drawing has no Project references: System.Drawing.Common <- System.Windows.Forms.Interop <- System.Windows.Forms System.Windows.Forms.Interop currently has CsWin32 code generation (all internal), has |
The The reason you're seeing these errors now is that after this change there are two referenced assemblies that defined As for how to move forward there are not a lot of good options. I could not find a good way to suppress the Longer term I think we likely need to revisit this behavior in XML doc processing. It's effectively implicit Note: Want to be clear I think CsWin23 code generation strategy is completely rationale here. The issue is XML docs having implicit IVT to everything. |
|
I experimented with adding an additional interop assembly to contain shared types so there would be no ambiguity. While I can create this and build it fine, XML comment crefs still can't figure out types that are only defined once in the referenced project. So: System.Windows.Primitives -> System.Windows.Forms.Primitives -> System.Windows->Forms
Note that in order to do this you have to pass the System.Windows.Primitives: System.Drawing.Common: |
|
I guess we have to wait for a solution from the compiler team? Maybe an escape hatch to only include internals when visible. |
|
You can resolve issues in The |
I think I tried this but had issues with Global Usings. |
|
@elachlan Can I push a small fix to this PR? |
|
@sharwell can you share what you are thinking first, instead of pushing directly? I'm hesitant to manually create aliases for every conflicting type. |
|
@JeremyKuhne I'm going to push the change based on the 👍 from @elachlan and the fact that this is a draft where it can be easily removed if you don't like it |
| @@ -18,6 +18,7 @@ | |||
| using static Interop; | |||
| using ComTypes = System.Runtime.InteropServices.ComTypes; | |||
| using Encoding = System.Text.Encoding; | |||
| using HandleRef_HWND = HandleRef<Windows.Win32.Foundation.HWND>; | |||
There was a problem hiding this comment.
📝 Limited choices here because HandleRef<T> is defined in the global namespace and dotnet/roslyn#116 doesn't appear to be moving forward
| global using HDC = Windows.Win32.Graphics.Gdi.HDC; | ||
| global using HGLOBAL = Windows.Win32.Foundation.HGLOBAL; | ||
| global using HWND = Windows.Win32.Foundation.HWND; | ||
| global using PCWSTR = Windows.Win32.Foundation.PCWSTR; |
There was a problem hiding this comment.
📝 Only need aliases for items that appear by name in cref attributes. Note that for each alias, there is also a global using directive for the containing namespace so the alias will never be accessible in a code location where the target of the alias would not already be accessible.
|
Closing in favor of #10598 |
Related: #9879
Microsoft Reviewers: Open in CodeFlow