Bug 213886

Summary: [WebXR] Retrieve WebGL framebuffer resolution from XR devices
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: New BugsAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, darin, dino, svillar, webkit-bug-importer, youennf, zan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 208988    
Attachments:
Description Flags
Patch
none
Patch cgarcia: review+

Description Sergio Villar Senin 2020-07-02 08:47:19 PDT
[WebXR] Retrieve WebGL framebuffer resolution from XR devices
Comment 1 Sergio Villar Senin 2020-07-02 09:08:03 PDT
Created attachment 403370 [details]
Patch
Comment 2 youenn fablet 2020-07-03 00:02:15 PDT
Comment on attachment 403370 [details]
Patch

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

> Source/WebCore/Modules/webxr/WebXRRenderState.h:66
> +    HTMLCanvasElement* m_outputCanvas { nullptr };

Can we use a WeakPtr<HTMLCanvasElement> instead?
It is not set in this patch so it is unclear how should we use it.
Also it seems this is only used for getting the size at the moment. In principle, we could store a size instead.

> Source/WebCore/platform/xr/PlatformXR.h:57
> +    using IntSize = WebCore::IntSize;

Why do we need to redefine it?

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:262
> +        XrResult result = xrEnumerateViewConfigurationViews(m_instance, m_systemId, configType, 0, &viewCount, nullptr);

auto?

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:278
> +        m_configurationViews.add(configType, configViews);

WTFMove(configViews)

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.cpp:284
> +    XrViewConfigurationType configType = mode == SessionMode::Inline ? XR_VIEW_CONFIGURATION_TYPE_PRIMARY_MONO : XR_VIEW_CONFIGURATION_TYPE_PRIMARY_STEREO;

auto?

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h:55
> +    using ViewConfigurationPropertiesMap = WTF::HashMap<XrViewConfigurationType, XrViewConfigurationProperties, WTF::IntHash<XrViewConfigurationType>, WTF::StrongEnumHashTraits<XrViewConfigurationType>>;

s/WTF:://

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h:57
> +    using ViewConfigurationViewsMap = WTF::HashMap<XrViewConfigurationType, Vector<XrViewConfigurationView>, WTF::IntHash<XrViewConfigurationType>, WTF::StrongEnumHashTraits<XrViewConfigurationType>>;

s/WTF:://
Comment 3 Sergio Villar Senin 2020-07-03 00:47:21 PDT
Created attachment 403446 [details]
Patch
Comment 4 Sergio Villar Senin 2020-07-09 02:24:42 PDT
(In reply to Sergio Villar Senin from comment #3)
> Created attachment 403446 [details]
> Patch

Kindly ping
Comment 5 Carlos Garcia Campos 2020-07-09 23:24:12 PDT
Comment on attachment 403446 [details]
Patch

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

> Source/WebCore/platform/xr/openxr/PlatformXROpenXR.h:26
> +#include <wtf/Optional.h>

What is this include for? I don't see any use of Optional in this header.
Comment 6 Sergio Villar Senin 2020-07-10 04:08:20 PDT
Committed r264215: <https://trac.webkit.org/changeset/264215>
Comment 7 Radar WebKit Bug Importer 2020-07-10 04:09:13 PDT
<rdar://problem/65328138>