Bug 213886 - [WebXR] Retrieve WebGL framebuffer resolution from XR devices
Summary: [WebXR] Retrieve WebGL framebuffer resolution from XR devices
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on:
Blocks: 208988
  Show dependency treegraph
 
Reported: 2020-07-02 08:47 PDT by Sergio Villar Senin
Modified: 2020-07-10 06:29 PDT (History)
7 users (show)

See Also:


Attachments
Patch (15.68 KB, patch)
2020-07-02 09:08 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (15.74 KB, patch)
2020-07-03 00:47 PDT, Sergio Villar Senin
cgarcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>