Bug 174202 – [WebVR] Add IDLs and stubs

• Component: New Bugs
• Status: RESOLVED FIXED

[Sergio Villar Senin]

[WebVR] Add IDLs and stubs

[Sergio Villar Senin]

Created attachment 314723 [details]
Patch

[Sergio Villar Senin]

It's missing the Xcode stuff, but perhaps could be added later by someone else.

[Alex Christensen]

Comment on attachment 314723 [details]
Patch

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

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:223
> +ENABLE_WEBVR = ENABLE_WEBVR;
> +ENABLE_OFFSCREEN_CANVAS = ENABLE_OFFSCREEN_CANVAS;

I'm not sure if we'll want to have these on by default during development.

> Source/WebCore/Modules/webvr/NavigatorWebVR.cpp:39
> +const Vector<RefPtr<VRDisplay>>& NavigatorWebVR::activeVRDisplays(Navigator&)

Could this return a const Vector<Ref<VRDisplay>>& ?

> Source/WebCore/Modules/webvr/VRStageParameters.idl:33
> +] interface VRStageParameters {
> +    readonly attribute Float32Array sittingToStandingTransform;
> +
> +    readonly attribute float sizeX;
> +    readonly attribute float sizeZ;
> +};

I guess this is more of a spec issue, but I've never been convinced that a stage is universally part of VR.  I understand the need for having two displays with two projection matrices and I think that's necessary for VR, but I'm not sure all VR applications want to define a 2D stage where the person is.  Was it really worth standardization?

[Sam Weinig]

This should probably be discussed on webkit-dev before moving forward.

[Sam Weinig]

Some notes:

- IDL 'unsigned long' maps to WebCore 'unsigned'.
- It would be better to split Offscreen Canvas and WebVR into separate changes.

[Sergio Villar Senin]

(In reply to Sam Weinig from comment #4)
> This should probably be discussed on webkit-dev before moving forward.

Oh right. I had the impression that it was already discussed but it was in the context of some private emails with some Apple folks.

[Sergio Villar Senin]

Created attachment 315247 [details]
Patch

Moved unsigned long to unsigned. Removed OffscreenCanvas stuff

[Sergio Villar Senin]

Maciej asked in the list to replace the build flag by a runtime flag. Makes total sense, I'll do it.

[Sergio Villar Senin]

Created attachment 317801 [details]
Patch

[Sergio Villar Senin]

Recent changes include
* Moved from build flag -> runtime flag
* Added a couple of basic test cases
* Fixed build without GAMEPAD support
* Added temptative Xcode stuff. Cannot make it build as some DerivedSources are not generated (will need some help from an Appler).

[Sergio Villar Senin]

Created attachment 317802 [details]
Patch

[Sergio Villar Senin]

I know we're in summer time, any reviewer?

Also could someone help with the Xcode stuff?

[Zan Dobersek]

Created attachment 319389 [details]
WIP patch

Attachment #317802 [details] with necessary XCode changes.

[Zan Dobersek]

(In reply to Zan Dobersek from comment #13)
> Created attachment 319389 [details]
> WIP patch
> 
> Attachment #317802 [details] with necessary XCode changes.

This builds now, but there's test failures for the two added tests.

Also, please don't forget to update the ChangeLog entries, a few of them are redundant now that the ENABLE_WEBVR flag isn't introduced anymore.

[Build Bot]

Comment on attachment 319389 [details]
WIP patch

Attachment 319389 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4412049

New failing tests:
webvr/webvr-enabled.html
webvr/webvr-disabled.html

[Build Bot]

Created attachment 319401 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 319389 [details]
WIP patch

Attachment 319389 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4412055

New failing tests:
webvr/webvr-enabled.html
webvr/webvr-disabled.html

[Build Bot]

Created attachment 319403 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 319389 [details]
WIP patch

Attachment 319389 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4412089

New failing tests:
webvr/webvr-enabled.html
webvr/webvr-disabled.html

[Build Bot]

Created attachment 319408 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 319389 [details]
WIP patch

Attachment 319389 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4412099

New failing tests:
webvr/webvr-enabled.html
webvr/webvr-disabled.html

[Build Bot]

Created attachment 319411 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Sergio Villar Senin]

Created attachment 319890 [details]
Patch

Tests working fine. Mac should build now too

[Sergio Villar Senin]

Ping reviewers. The patch is getting rotten :)

[Dean Jackson]

Comment on attachment 319890 [details]
Patch

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

It's unclear to me if we should implement this version (1.1) or the next version (2.0). Version 1.1 says it is already deprecated. Version 2.0 says it isn't ready to implement yet. I guess we should start with 1.1.

> Source/WebKit/UIProcess/API/C/WKPreferences.cpp:1881
> +
> +void WKPreferencesSetWebVREnabled(WKPreferencesRef preferencesRef, bool flag)
> +{
> +    toImpl(preferencesRef)->setWebVREnabled(flag);
> +}
> +
> +bool WKPreferencesGetWebVREnabled(WKPreferencesRef preferencesRef)
> +{
> +    return toImpl(preferencesRef)->webVREnabled();
> +}

I don't think we need the C API entry points.

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:531
> +// Defaults to false.
> +WK_EXPORT void WKPreferencesSetWebVREnabled(WKPreferencesRef, bool flag);
> +WK_EXPORT bool WKPreferencesGetWebVREnabled(WKPreferencesRef);
> +

Ditto.

[Sergio Villar Senin]

(In reply to Dean Jackson from comment #25)
> Comment on attachment 319890 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319890&action=review
> 
> It's unclear to me if we should implement this version (1.1) or the next
> version (2.0). Version 1.1 says it is already deprecated. Version 2.0 says
> it isn't ready to implement yet. I guess we should start with 1.1.

I just followed the "do what other browsers do" "rule". Everybody else is shipping 1.1. It's true that the specs are tagged as deprecated but 2.0 is not a thing yet. Also the amount of API changes between 1.1 and 2.0 is not that big, I expect the transition not to be overwhelmingly difficult.

[Sergio Villar Senin]

Committed r221966: <http://trac.webkit.org/changeset/221966>

[Radar WebKit Bug Importer]

<rdar://problem/34694508>