WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
134325
Add new platform gamepad abstractions
https://bugs.webkit.org/show_bug.cgi?id=134325
Summary
Add new platform gamepad abstractions
Brady Eidson
Reported
2014-06-25 19:24:27 PDT
Add new platform gamepad abstractions These include a PlatformGamepad object and platform strategies to support either in-process or out-of-process gamepad input. The code is basically unused here, but goes hand-in-hand with the patch that will be reviewed separately in
https://bugs.webkit.org/show_bug.cgi?id=134324
Attachments
Patch v1
(35.88 KB, patch)
2014-06-25 19:31 PDT
,
Brady Eidson
dino
: review+
Details
Formatted Diff
Diff
Patch v2 - Patch that should apply
(33.85 KB, patch)
2014-06-25 19:41 PDT
,
Brady Eidson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2014-06-25 19:31:47 PDT
Created
attachment 233864
[details]
Patch v1
Dean Jackson
Comment 2
2014-06-25 19:41:13 PDT
Comment on
attachment 233864
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=233864&action=review
> Source/WebCore/WebCore.xcodeproj/project.pbxproj:7212 > - 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ISOVTTCue.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > - 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ISOVTTCue.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > + 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > + 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ISOVTTCue.h; sourceTree = "<group>"; };
Is this intentional?
> Source/WebCore/platform/GamepadStrategy.h:50 > +#endif // ENABLE(GAMEPAD)
Nit: I think we agreed to not bother with these style comments
> Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:594 > +#endif // PLATFORM(IOS) > > -#endif > +#endif // PLATFORM(COCOA)
But here you're adding them, so maybe my memory is incorrect and they should be around :)
Brady Eidson
Comment 3
2014-06-25 19:41:44 PDT
Created
attachment 233866
[details]
Patch v2 - Patch that should apply
Brady Eidson
Comment 4
2014-06-25 19:43:23 PDT
(In reply to
comment #2
)
> (From update of
attachment 233864
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=233864&action=review
> > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:7212 > > - 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ISOVTTCue.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > > - 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ISOVTTCue.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > > + 07AC46FF1952102100EE9723 /* ISOVTTCue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ISOVTTCue.cpp; sourceTree = "<group>"; }; > > + 07AC47001952102100EE9723 /* ISOVTTCue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ISOVTTCue.h; sourceTree = "<group>"; }; > > Is this intentional?
Nope, and it's gone in the updated patch.
> > > Source/WebCore/platform/GamepadStrategy.h:50 > > +#endif // ENABLE(GAMEPAD) > > Nit: I think we agreed to not bother with these style comments
I think we agreed to not bother *always* especially when they're short blocks where it would be pointless. In this case...
> > > Source/WebKit2/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:594 > > +#endif // PLATFORM(IOS) > > > > -#endif > > +#endif // PLATFORM(COCOA) > > But here you're adding them, so maybe my memory is incorrect and they should be around :)
I added them because I couldn't follow what these blocks were! Thanks for the review!
Brady Eidson
Comment 5
2014-06-25 20:59:46 PDT
http://trac.webkit.org/changeset/170459
Sam Weinig
Comment 6
2014-06-25 21:54:03 PDT
We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do.
Michal Debski
Comment 7
2014-06-26 03:14:49 PDT
(In reply to
comment #6
)
> We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do.
I have already updated Gamepad API and did not use PlatformStrategies, please take a look at my patch and tell me what do you think about this change:
https://bugs.webkit.org/show_bug.cgi?id=133850
Brady Eidson
Comment 8
2014-06-26 08:08:12 PDT
(In reply to
comment #6
)
> We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do.
I have a good non-platformStrategies based solution for this I'll get in today.
Brady Eidson
Comment 9
2014-06-26 08:10:13 PDT
(In reply to
comment #7
)
> (In reply to
comment #6
) > > We are trying not to add any more PlatformStrategies, and instead are aiming to remove them. Please back this out and let's discuss other mechanisms you can use to achieve what it is you are trying to do. > > I have already updated Gamepad API and did not use PlatformStrategies, please take a look at my patch and tell me what do you think about this change: >
https://bugs.webkit.org/show_bug.cgi?id=133850
Wow, coincidental! Unfortunately there's a lot of needs the work you've done there don't fulfill. For example, your patch doesn't allow for WebKit2 implementations (which is the very reason I used platform strategies in the first place) I'll comment more in your bug(s) later today. But I bet we can use things like your layout test work going forward!
Brady Eidson
Comment 10
2014-06-26 09:53:10 PDT
Filed a new bug to remove the use of PlatformStrategies (
https://bugs.webkit.org/show_bug.cgi?id=134348
), re-resolving this one.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug