Repository navigation
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 |
32a0b77 to
4710445
Compare
Marijn Suijten (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 |
4710445 to
b28a105
Compare
b28a105 to
c29d3a6
Compare
|
I think these are the remaining items that need conversion? |
|
Rafael Rivera (@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. |
|
Rafael Rivera (@riverar) so that means there's a definitive yes on annotating pointer types? |
|
Yup, lets do it 🚀 |
Marijn Suijten (MarijnS95)
changed the title
c29d3a6 to
33f3315
Compare
|
Rafael Rivera (@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? |
|
Rafael Rivera (@riverar) I'll rebase this when you or Mike Battista (@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 |
33f3315 to
f19676f
Compare
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? |
ea4f255 to
dc191a0
Compare
dc191a0 to
d34174c
Compare
d34174c to
2795eef
Compare
2795eef to
dc24188
Compare
dc24188 to
ce74355
Compare
…onding enum type These are more convenient and descriptive to use than plain `UINT`s.
162472a to
63ee44d
Compare
63ee44d to
1340097
Compare
|
Thanks for the review Rafael Rivera (@riverar)! Mike Battista (@mikebattista) let me know when this is ready to be merged, so that I can solve the conflicts with |
372c4b8
|
Merged. Thanks! |
Marijn Suijten (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.
Rafael Rivera (@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, ...)
Rafael Rivera (@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.
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.
These are more convenient and descriptive to use than plain
UINTs.