Bug 134381 - HIDGamepads should populate themselves with initial input values
Summary: HIDGamepads should populate themselves with initial input values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 134076
  Show dependency treegraph
 
Reported: 2014-06-26 20:58 PDT by Brady Eidson
Modified: 2014-06-27 13:53 PDT (History)
6 users (show)

See Also:


Attachments
Patch v1 (3.33 KB, patch)
2014-06-26 22:27 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v2 - Now with 100% more ChangeLog (4.26 KB, patch)
2014-06-26 22:30 PDT, Brady Eidson
darin: review+
beidson: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2014-06-26 20:58:12 PDT
HIDGamepads should populate themselves with initial input values.

At construction time, for each element they own they should query the HIDElement on the device.
Comment 1 Brady Eidson 2014-06-26 22:27:07 PDT
Created attachment 233961 [details]
Patch v1
Comment 2 Brady Eidson 2014-06-26 22:30:26 PDT
Created attachment 233962 [details]
Patch v2 - Now with 100% more ChangeLog
Comment 3 Brady Eidson 2014-06-26 23:02:33 PDT
Ignore the can't-buildness that EWS sees - In my tree this patch is built on previous patches.  I will pass it through EWS before landing.
Comment 4 Darin Adler 2014-06-27 09:54:45 PDT
Comment on attachment 233962 [details]
Patch v2 - Now with 100% more ChangeLog

View in context: https://bugs.webkit.org/attachment.cgi?id=233962&action=review

Patch didn’t apply in EWS.

> Source/WebCore/platform/mac/HIDGamepad.cpp:75
> +    IOHIDValueRef value;

I think a local just before each call to IOHIDDeviceGetValue would be nicer than sharing this local.

> Source/WebCore/platform/mac/HIDGamepad.cpp:79
> +        IOHIDElementRef element = button->iohidElement.get();
> +        if (IOHIDDeviceGetValue(IOHIDElementGetDevice(element), element, &value) == kIOReturnSuccess)
> +            valueChanged(value);

I know this sounds a little silly, but a helper that takes an const HIDGamepadElement& and does all this would make the code tigther and easier to read.

> Source/WebCore/platform/mac/HIDGamepad.h:80
>  struct HIDGamepadAxis : public HIDGamepadElement {

Normally with struct inheritance we would omit the "public" (it's the default).
Comment 5 Brady Eidson 2014-06-27 13:53:59 PDT
http://trac.webkit.org/changeset/170551