dxgi: Override enum (flags) ABI type to UINT when used in structures

[MarijnS95]

CC @riverar

Even though my previous contribution in #1757 added AssociatedEnum annotations to both function parameters and structure fields for some enum types, language bindings like windows-rs can only generate ABI conversions for parameters (and return types) inside functions when there's a mismatch (untyped enums are assumed to be INT, whereas structures and functions usually use UINT instead of the enum name). There is no place for this in repr(C) structs (in Rust), requiring us to get more creative to use the original UINT ABI while still replacing the field type with that of the relevant self-descriptive enum type.

Instead, move these (also rather disconnected) annotations and references on those enums and their respective fields to enums.json, where we can set the AssociatedEnum attribute at once in a more coherent way while above all allowing us to override the enum/flags ABI type to match how it's used by the APIs in function parameters and struct fields.

Also, DXGI_OUTDUPL_FLAG is now used as the type for IDXGIOutput5::DupliucateOutput1::Flags. The documentation as DXGI_OUTDUPL_COMPOSITED_UI_CAPTURE_ONLY is a valid value. This is now corrected at microsoft/DirectX-Headers#128.

TODO

This is my attempt at solving #1757 (comment) and microsoft/windows-rs#3044 (comment), but as is obvious from the changes since the last release it is not working yet. Unlike other enums that I added to enums.json with for example autoPopulate, none of these configs appear to be working. Any obvious thing that I'm doing wrong?

Then:

• Should we convert all remaining enum flags (under --with-attribute) to use enums.json (I skipped all those that are only used in function parameters)?
• Should their ABI be updated to UINT as well (e.g. to preempt being used in structs later on)?

[MarijnS95]

@riverar since you pointed this out to me in microsoft/windows-rs#3044 (comment), can you or @mikebattista help me with this?

As can be seen in the changelog in this PR, nothing is changing, presumably because the scraper already "has" an enum (whereas enums.json seems to mainly be used to scrape existing constants into enums).

Fortunately there seems to be a addUsesTo to "extend" an existing enum, but it doesn't seem to allow us to change the type:

EMITWINMD : error : Failed to emit DXGI_SWAP_CHAIN_DESC in win32metadata\generation\WinSDK\obj\generated\obj\crossarch\common\Dxgi.modified.cs: DXGI_SWAP_CHAIN_DESC.Flags was remapped to enum Windows.Win32.Graphics.Dxgi.DXGI_SWAP_CHAIN_FLAG (type int, size 4) but the original field was of type UINT (size 4). Either don't use an enum or make sure the enum is of the same size and sign. [generation\WinSDK\Windows.Win32.proj]

[MarijnS95]

@riverar any suggestion on the above? I'd love to get this finished :)

[MarijnS95]

I moved this into generation/WinSDK/Partitions/Dxgi/settings.rsp with --enumMakeFlags but the [Flags] attribute disappears. Is it only expected to work inside emitter.settings.rsp? Other flags (--with-type, --with-attribute do work there).

What's preferred:

1. Have the types under --enumMakeFlags in emitter.settings.rsp
2. Have TYPE=Flags under --with-atribute in the Dxgi-specific settings.rsp above?

[MarijnS95]

--memberRemap also doesn't seem to have an effect in generation/WinSDK/Partitions/Dxgi/settings.rsp, only in generation/WinSDK/emitter.settings.rsp.

[MarijnS95]

What do folks think, should we proactively convert other enum types - that aren't used in structs just yet - to uint as well to use them as a direct type instead of via AssociatedEnum?

@kennykerr we've seen it before in microsoft/windows-rs#3111 that AssociatedEnum like this cause impl wrappers to borrow instead of pass by (cheaply copyable) value. Is that something you have on your radar to fix in windows-rs?

[MarijnS95]

What do folks think, should we proactively convert other enum types - that aren't used in structs just yet - to uint as well to use them as a direct type instead of via AssociatedEnum?

I've just pushed this and regenerated windows-rs at MarijnS95/windows-rs@fbd504a. Notable takeaways:

• AssociatedEnum doesn't seem to work for to [out] pointers, for example CheckHardwareCompositionSupport() was still returning u32 but now finally returns DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS;
• All flags: & borrows on _Impl traits are gone, and now passed by values;
• (less relevant: less internal conversions; raw function signatures now use the underlying flags type more early).

[riverar]

I'll have to re-familiarize myself with the context here, but one correction is that DXGI_ADAPTER_FLAG and friends are not uint in the headers.

[MarijnS95]

@riverar the contxt here is microsoft/windows-rs#3044 (comment) and our conversation following it. I don't think you ever replied to my final request for clarification though, but this PR is implementing point 2. fwiw.

[riverar]

Right, these are int, not uint.

typedef 
enum DXGI_ADAPTER_FLAG
    {
        DXGI_ADAPTER_FLAG_NONE	= 0,
        DXGI_ADAPTER_FLAG_REMOTE	= 1,
        DXGI_ADAPTER_FLAG_SOFTWARE	= 2,
        DXGI_ADAPTER_FLAG_FORCE_DWORD	= 0xffffffff
    } 	DXGI_ADAPTER_FLAG;

[MarijnS95]

@riverar but also microsoft/DirectX-Headers#128 (comment).

Instead of only discussing ABI here, can we come up with any solution that allows us to have correct enum annotations regardless of whether the accidental default int enum type mismatches with the uint Flags; type used in various structs and function signatures?

[riverar]

So this PR appears to be trying to do two things:

1. Work around the DirectX API parameter type and enum mismatches (uint vs int). I don't believe there's anything more that metadata can do here. Aligning the types may not impact ABI, but it does impact our attempts to preserve source compatibility.

   Here's what we have today:

   [Flags]
   public enum DXGI_ADAPTER_FLAG // .field public specialname rtspecialname int32 value__
   {
       DXGI_ADAPTER_FLAG_NONE = 0,
       DXGI_ADAPTER_FLAG_REMOTE = 1,
       DXGI_ADAPTER_FLAG_SOFTWARE = 2
   }
   
   public struct DXGI_ADAPTER_DESC1
   {
   ...
       [AssociatedEnum("DXGI_ADAPTER_FLAG")]
       public uint Flags;
   }

2. Associate additional struct fields and method parameters with existing enums. That's cool and would be received well in a separate PR. (e.g. IDXGIOutput6::CheckHardwareCompositionSupport::pFlags = [AssociatedEnum("DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS")]).

[riverar]

@riverar but also microsoft/DirectX-Headers#128 (comment).

Instead of only discussing ABI here, can we come up with any solution that allows us to have correct enum annotations regardless of whether the accidental default int enum type mismatches with the uint Flags; type used in various structs and function signatures?

We emit the original types (from the headers) and a helpful attribute to help bridge the mismatch as shown above. I think there's enough information in metadata for projections to handle this. I believe Rust can emit a function that accepts IntoParam<UINT> + IntoParam<DXGI_EXAMPLE_FLAGS>. And C# can emit overloaded methods.

[MarijnS95]

@riverar for 2. no that is not correct. CheckHardwareCompositionSupport::pFlags already had this annotation from #1757. In Windows-rs it just doesn't work when the pointer is translated to a return value through its [Out] parameter, which probably warrants a bug report over there?

---

For the new IDXGIOutput5::DuplicateOutput1::Flags=[AssociatedEnum("DXGI_OUTDUPL_FLAG")] (and alphabetical sorting) I can open a separate PR. But before that, what is your thought on the above, using --with-attribute EXAMPLE_FLAG=Flags in TheNamespace/settings.rsp versus --enumMakeFlags\nEXAMPLE_FLAGS\n in emitter.settings.rsp?

---

Rust already emits functions that take DXGI_EXAMPLE_FLAGS and internally convert flag.0 as u32 thanks to AssociatedEnum. This just doesn't work for struct fields (and return values it seems). That is something we should seek to address, in my opinion. Solving the odd borrows on _Impl traits is bonus.

---

As a side-note, while reworking this PR, I found the following assignments which are also mutating ABI:

D3D11CreateDevice::Flags=D3D11_CREATE_DEVICE_FLAG
D3D11CreateDeviceAndSwapChain::Flags=D3D11_CREATE_DEVICE_FLAG

There are probably more.

[riverar]

@riverar for 2. no that is not correct. CheckHardwareCompositionSupport::pFlags already had this annotation from #1757. In Windows-rs it just doesn't work when the pointer is translated to a return value through its [Out] parameter, which probably warrants a bug report over there?

Ah, yup, I misread the diff. Will scribble that out in the original post.

In Windows-rs it just doesn't work when the pointer is translated to a return value through its [Out] parameter, which probably warrants a bug report over there? [...] This just doesn't work for struct fields (and return values it seems). That is something we should seek to address, in my opinion. Solving the odd borrows on _Impl traits is bonus.

Looks like that's a known limitation at this time (see microsoft/windows-rs#2797). Now that newer v61 metadata has been incorporated into windows-rs, feel free to put in an enhancement request for this. It may help your request to point out that this metadata now contains associated enums on out params as such:

unsafe HRESULT CheckOverlayColorSpaceSupport(
  [In] DXGI_FORMAT Format,
  [In] DXGI_COLOR_SPACE_TYPE ColorSpace,
  [In] IUnknown pConcernedDevice,
  [AssociatedEnum("DXGI_OVERLAY_SUPPORT_FLAG"),Out] uint* pFlags); // <---

what is your thought on the above, using --with-attribute EXAMPLE_FLAG=Flags in TheNamespace/settings.rsp versus --enumMakeFlags\nEXAMPLE_FLAGS\n in emitter.settings.rsp?

I like the idea of keeping settings in their respective partitions. If that’s not working, we can investigate, but it’s not essential. Just choose one that works, and we can refine it later. For now, it’s just confusing chatter.

As a side-note, while reworking this PR, I found the following assignments which are also mutating ABI: [...]

I have called the police, thanks. Looks like that was missed as part of this effort #1572. We can fix that in a separate PR too.

[MarijnS95]

Now that newer v61 metadata has been incorporated into windows-rs, feel free to put in an enhancement request for this.

I have already done this a long time ago, but it was closed.

As a side-note, while reworking this PR, I found the following assignments which are also mutating ABI: [...]

I have called the police, thanks. Looks like that was missed as part of this effort #1572. We can fix that in a separate PR too.

Wasn't there a compile-time error for this in the past?