Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Windows] Incorrect report size calculation #102

Open
codembed opened this issue Oct 11, 2022 · 3 comments
Open

[Windows] Incorrect report size calculation #102

codembed opened this issue Oct 11, 2022 · 3 comments

Comments

@codembed
Copy link

codembed commented Oct 11, 2022 •
edited
Loading

For each report in a collection, more than one field type is included in the calculation of the report size, however the calculation should be filtered by field type

For example, for a definition similar to:

Usage Page (Generic Desktop Ctrls)
Usage (Mouse)
Collection (Application)
	Usage ()
	Collection (Logical)
		Usage Page ()
		Report ID (1)
		Logical Minimum (0)
		Logical Maximum (127)
		Report Count (1)
		Report Size (8)
		Input (Data,Var,Abs)
		Feature (Data,Var,Abs)
		Output (Data,Var,Abs)
	End Collection
End Collection

The actual report sizes (including id byte) are
InputReport(ID_1) -> 2 bytes
OutputReport(ID_1) -> 2 bytes
FeatureReport(ID_1) -> 2 bytes

However, the size is incorrectly described as 4 bytes for each report (1-id 1-feature 1-output 1-input)

Chromium appears to pad the collection definitions with additional 'const' bytes, and the actual inputReport event data also contains trailing zero padding, which is not present over the wire.

The unexpected padding can be observed in webhid-explorer as extra 'Cnst,Ary,Abs' bits near the end of each report description.

For inputs, there is likely minimal impact, however Feature and Output reports that are created according to the invalid definition could be rejected (typically by the OS, or perhaps the device?). I have not tested this.
@codembed
Copy link
Author

So, observed this on Windows (10), but Chrome OS did not have this problem and has correct report sizes.

@codembed codembed changed the title Incorrect report size calculation for report id having more than one field type (Input/Output/Feature) [Windows] Incorrect report size calculation for report id having more than one field type (Input/Output/Feature) Oct 11, 2022
@codembed
Copy link
Author

My assumptions in the opening comment are misleading.

It looks like this is the same as #86

On closer inspection, it seems that all reports of one type have a size equal to the maximum for that type, which is not because of common IDs.

So, if I have one input report size = 12 and one size = 4, both are reported with length 12
Two feature reports with size 2 and 7 bytes are both described with length 7, etc.

Since the characteristics of each report field is known, why can't the correct size be calculated for each report and then reflected in HIDDevice.collections?

@codembed codembed changed the title [Windows] Incorrect report size calculation for report id having more than one field type (Input/Output/Feature) [Windows] Incorrect report size calculation Oct 11, 2022
@nondebug
Copy link
Collaborator

On closer inspection, it seems that all reports of one type have a size equal to the maximum for that type, which is not because of common IDs.

This is a platform limitation on Windows where the size of the report buffer is required to match the maximum size of a report of that type (input, output, or feature). WebHID API reflects this requirement by inserting a padding item at the end of the HIDDevice.collections array that extends each report to the maximum length.

I'd like to remove the limitation and have WebHID return the same collection information on Windows that we return on other platforms. However, this doesn't appear to be possible using the information exposed through the Windows HID API. On Linux, macOS, and ChromeOS we read the HID report descriptor from the device and parse it to learn the sizes of each report. On Windows the report descriptor isn't accessible to userspace applications. Instead, Windows provides the information from the report descriptor as an opaque object HIDP_PREPARSED_DATA which can be passed to other API methods used for reading and writing report fields.

Chrome inspects the opaque HIDP_PREPARSED_DATA object and uses it to reconstruct the layout of each report. The preparsed object loses some information from the original report descriptor, so the reconstructed reports are not exactly the same as what we get on other platforms. Details like const padding items and report lengths are not preserved.

Since the characteristics of each report field is known, why can't the correct size be calculated for each report and then reflected in HIDDevice.collections?

The correct report size is always the sum of reportSize * reportCount for each item in HIDReportInfo.items. If you use this size to allocate report buffers you'll automatically include the extra padding on Windows.

I'm in favor of adding HIDReportInfo.byteLength or similar as a convenience but it's not high on my list of priorities. A pull request to add it to the spec would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@codembed @nondebug