microsoft / microsoft/win32metadata

DIPROP_* constants don't match Win32 API

Open
#1,720 6 comments 0 reactions 1 assignee Claimed by @mikebattista View on GitHub
broken api usability
Dominant language
C++
Stars
1.5k
Forks
149
Avg merge
5d 16h
Merged PRs (30d)
4

Description

### Suggestion

The DIPROP_* constants (from DirectInput) are defined in the Rust bindings in a fundamentally different way than they are in the original Win32 SDK.

In the original header (dinput.h), for C code they are defined as such (simplified for brevity):

```c
#define MAKEDIPROP(prop) ((const GUID *)(prop))

#define DIPROP_BUFFERSIZE MAKEDIPROP(1)
#define DIPROP_AXISMODE MAKEDIPROP(2)
#define DIPROP_GRANULARITY MAKEDIPROP(3)
#define DIPROP_RANGE MAKEDIPROP(4)
...
```

Note that these constants do not represent actual GUIDs, but numeric values cast as a pointer to a GUID (`const GUID *`). For C++, they are additionally dereferenced to make a C++ reference rather than a pointer, but are fundamentally the same.

In the Rust bindings, they are defined as such (simplified for comparison's sake):
```rust
pub const DIPROP_BUFFERSIZE: ::windows_core::GUID = ::windows_core::GUID::from_u128(1);
pub const DIPROP_AXISMODE: ::windows_core::GUID = ::windows_core::GUID::from_u128(2);
pub const DIPROP_GRANULARITY: ::windows_core::GUID = ::windows_core::GUID::from_u128(3);
pub const DIPROP_RANGE: ::windows_core::GUID = ::windows_core::GUID::from_u128(4);
...
```

In the Rust binding, they are actual GUIDs whose contents are derived from the associated numeric values. Their meaning (and usage) is different than with the original SDK.

These constants are intended to be passed to the DirectInput `GetProperty` and `SetProperty` methods, which both have an argument that takes a `const GUID *`/`*const GUID`. I am not currently aware of any other uses of these constants. From [an MSDN example](https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee417908(v=vs.85)):
```c
hr = idirectinputdevice9_GetProperty(pdid, DIPROP_BUFFERSIZE, &dipdw.diph);
```

Due to the mismatch, rather than being able to pass the constant directly in, the constants have to be shuffled around in order to function as intended. An equivalent example using the Rust bindings would have to be:
```rust
hr = pdid.GetProperty(DIPROP_BUFFERSIZE.to_u128() as *const GUID, &mut dipdw.diph as *mut DIPROPHEADER);
```

As a side effect, at a first glance at the Rust API, one might assume that they would be used differently:
```rust
hr = pdid.GetProperty(&DIPROP_BUFFERSIZE as *const GUID, &mut dipdw.diph as *mut DIPROPHEADER);
```

This is safe, but results in a the function returning an error (`DIERR_UNSUPPORTED`/`E_NOTIMPL`) as these GUIDs are not handled by DirectInput and it isn't immediately obvious why.

It seems to me that the DIPROP_* constants should be defined as:

```rust
pub const DIPROP_BUFFERSIZE: *const ::windows_core::GUID = 1 as *const ::windows_core::GUID;
pub const DIPROP_AXISMODE: *const ::windows_core::GUID = 2 as *const ::windows_core::GUID;
pub const DIPROP_GRANULARITY: *const ::windows_core::GUID = 3 as *const ::windows_core::GUID;
pub const DIPROP_RANGE: *const ::windows_core::GUID = 4 as *const ::windows_core::GUID;
...
```

I feel that this more closely matches the original intent and usage of these constants, though any change would be a breaking change to any code currently using them. As far as I can reason it seems like it should result in a compile error for most reasonable usages, rather than resulting in incorrect behavior, but I'm not sure if there would be any potentially unsafe results from changing the types of these constants.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.