Associate DXGI function parameters and struct fields with the corresponding enum type #1757
Conversation
|
Before merging this, maybe we should do a bigger pass over this namespace and check some more |
MarijnS95
changed the title
|
This is basically a followup of #1583 where I should have checked for more types (even ones on the same struct of a different version...). Found a few more, in the https://learn.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-present Curiously in the linked PR we had to annotate these via |
|
I think these are the remaining items that need conversion? |
|
@riverar don't forget about: Those four are the only pointer params in this PR. |
|
Right, just looking for you to fix up these last four? -IDXGIOutput3::CheckOverlaySupport::pFlags=DXGI_OVERLAY_SUPPORT_FLAG
+IDXGIOutput3::CheckOverlaySupport::pFlags=[AssociatedEnum(DXGI_OVERLAY_SUPPORT_FLAG)]
-IDXGIOutput4::CheckOverlayColorSpaceSupport::pFlags=DXGI_OVERLAY_COLOR_SPACE_SUPPORT_FLAG
+IDXGIOutput4::CheckOverlayColorSpaceSupport::pFlags=[AssociatedEnum(DXGI_OVERLAY_COLOR_SPACE_SUPPORT_FLAG)
-IDXGIOutput6::CheckHardwareCompositionSupport::pFlags=DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS
+IDXGIOutput6::CheckHardwareCompositionSupport::pFlags=[AssociatedEnum(DXGI_HARDWARE_COMPOSITION_SUPPORT_FLAGS)
-IDXGISwapChain3::CheckColorSpaceSupport::pColorSpaceSupport=DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG
+IDXGISwapChain3::CheckColorSpaceSupport::pColorSpaceSupport=[AssociatedEnum(DXGI_SWAP_CHAIN_COLOR_SPACE_SUPPORT_FLAG)Edit: Just saw the last one hiding in there. Added above. |
|
@riverar so that means there's a definitive yes on annotating pointer types? |
|
Yup, lets do it 🚀 |
MarijnS95
changed the title
|
@riverar done! Would you please make sure that I've covered every remaining untyped Perhaps we should seek for the same patterns in D3D12? |
|
#1757 (comment) if you run into any of these: maybe we can turn the constants into enums? |
|
@riverar I'll rebase this when you or @mikebattista have reviewed it and are ready to merge it, to prevent constant force-pushes. Looking forward to the heads-up, thanks! |
|
Looks like it will also be advantageous to mark some of these types, like |
Since I ran into Again, please help me check that I've caught mostly everything! Documentation annotations are missing though, should those be included as well? |
MarijnS95
force-pushed
the
dxgi
branch
2 times, most recently
from
ea4f255 to
dc191a0
Compare
…onding enum type These are more convenient and descriptive to use than plain `UINT`s.
|
Thanks for the review @riverar! @mikebattista let me know when this is ready to be merged, so that I can solve the conflicts with |
|
Merged. Thanks! |
MarijnS95
changed the title
| --with-attribute | ||
| DXGI_MULTIPLANE_OVERLAY_YCbCr_FLAGS=Flags | ||
| DXGI_OFFER_RESOURCE_FLAGS=Flags | ||
| DXGI_OUTDUPL_FLAG=Flags |
There was a problem hiding this comment.
This flag has one DXGI_OUTDUPL_COMPOSITED_UI_CAPTURE_ONLY = 1 constant, but I couldn't find in what API it is supposed to be used. Only the constructor for output duplication has as Flags field that "should always be zero":
https://learn.microsoft.com/en-us/windows/win32/api/dxgi1_5/nf-dxgi1_5-idxgioutput5-duplicateoutput1
There was a problem hiding this comment.
Looks like a question for the DX repo. Suspect this is was meant to stay internal. But a real world example from OS components verifies your claim: pObject->DuplicateOutput1(device, DXGI_OUTDUPL_COMPOSITED_UI_CAPTURE_ONLY, ...)
There was a problem hiding this comment.
Sounds good, in that case the documentation should be updated too.
What's the desired way to assign the enum, emitter.settings.rsp or listing it in enums.json?
There was a problem hiding this comment.
Think we prefer enums.json these days!
There was a problem hiding this comment.
@riverar that might mean we could move more of the --with-attribute here - with explicit reassignments in emitter.settings.rsp - and consolidate them into enums.json?
Looks like there's also --enumMakeFlags in emitter.settings.rsp doing a similar thing 😅
There was a problem hiding this comment.
But a real world example from OS components verifies your claim:
pObject->DuplicateOutput1(device, DXGI_OUTDUPL_COMPOSITED_UI_CAPTURE_ONLY, ...)
@riverar where'd you find this example?
There was a problem hiding this comment.
bcastdvruserservice.dll!Windows::Media::Capture::Internal::DDACapture::AcquireOutputDuplication (26217.5000), reverse engineered 😅
There was a problem hiding this comment.
Posted the question at microsoft/DirectX-Headers#128, let's see.
Even though my previous contribution in microsoft#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.
Even though my previous contribution in microsoft#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.
Even though my previous contribution in microsoft#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.
Even though my previous contribution in microsoft#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.
These are more convenient and descriptive to use than plain
UINTs.