Bug 134381

Summary: HIDGamepads should populate themselves with initial input values
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Severity: Normal CC: bunhere, cdumez, commit-queue, dino, gyuyoung.kim, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: All   
Bug Depends on:    
Bug Blocks: 134076    
Description Flags
Patch v1
Patch v2 - Now with 100% more ChangeLog darin: review+, beidson: commit-queue?

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