[OpenXR] Hit testing returns empty results mixed with actual data

[svillar]

df0854f

[OpenXR] Hit testing returns empty results mixed with actual data
https://bugs.webkit.org/show_bug.cgi?id=305641

Reviewed by Fujii Hironori and Dan Glastonbury.

The OpenXRHitTestManager can call xrRaycastANDROID with specific
parameters to first determine how many results are available and then
use that number to size the Vector where the actual results will be
stored. This way we don't need to ask always for the max allowed number
of results (most of them would be empty/zero) but instead for the exact
number of available hit test results.

No new tests as this is specific to OpenXR.

* Source/WebKit/UIProcess/XR/openxr/OpenXRHitTestManager.cpp:
(WebKit::OpenXRHitTestManager::requestHitTest):

Canonical link: https://commits.webkit.org/305809@main

4f74b4c

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 win
	🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2	🧪 win-tests
✅ 🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	✅ 🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 api-mac-debug	✅ 🛠 wpe-cairo-libwebrtc
	🧪 api-ios		✅ 🛠 gtk
	🛠 vision	🧪 mac-wk2	🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	⏳ 🧪 vision-wk2	🧪 mac-wk2-stress	🛠 playstation
	✅ 🛠 tv	🧪 mac-intel-wk2
	✅ 🛠 tv-sim	✅ 🛠 mac-safer-cpp
	🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 31a5281)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	❌ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 api-mac-debug	✅ 🛠 wpe-cairo-libwebrtc
	✅ 🧪 api-ios		✅ 🛠 gtk
⏳ 🛠 🧪 jsc-debug-arm64	✅ 🛠 vision	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-wk2-stress	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv-sim	✅ 🛠 mac-safer-cpp
	✅ 🛠 watch
	✅ 🛠 watch-sim

[djg]

[fujii]

Don't you change constexpr int maxHitTestResults = 2 to increase the number? If you don't change, I don't understand what this change means.

[fujii]

Ah, I understand the problem.

    for (const auto xrResult : xrResults)
        results.append(XrPosefToPose(xrResult.pose));

This code always copies two items. It should check xrHitResults.resultsCountOutput.
I think the following change should be enough.

    for (auto i = 0; i < xrHitResults.resultsCountOutput; i++)
        results.append(XrPosefToPose(xrResults[i].pose));

If you want to use Vector for xrResults, we shouldn't use for loop here.
Vector::map is the efficient way to do the same.

[svillar]

Ah, I understand the problem.

    for (const auto xrResult : xrResults)
        results.append(XrPosefToPose(xrResult.pose));

This code always copies two items. It should check xrHitResults.resultsCountOutput. I think the following change should be enough.

    for (auto i = 0; i < xrHitResults.resultsCountOutput; i++)
        results.append(XrPosefToPose(xrResults[i].pose));

That would be fine but you'd still be allocating a vector of size maxHitTestResults which is larger than the one required, I know it is not a big deal because the number of results is small.

If you want to use Vector for xrResults, we shouldn't use for loop here. Vector::map is the efficient way to do the same.

OK will change that.

[webkit-early-warning-system]

EWS run on previous version of this PR (hash f11b888)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	🛠 win
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	🧪 win-tests
✅ 🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	❌ 🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 api-mac-debug	❌ 🛠 wpe-cairo-libwebrtc
	🧪 api-ios		❌ 🛠 gtk
	✅ 🛠 vision	🧪 mac-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	🧪 vision-wk2	🧪 mac-wk2-stress	🛠 playstation
	✅ 🛠 tv	🧪 mac-intel-wk2
	✅ 🛠 tv-sim	✅ 🛠 mac-safer-cpp
	🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on current version of this PR (hash 4f74b4c)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	🛠 win
	🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2	🧪 win-tests
✅ 🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	✅ 🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 api-mac-debug	✅ 🛠 wpe-cairo-libwebrtc
	🧪 api-ios		✅ 🛠 gtk
	🛠 vision	🧪 mac-wk2	🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	⏳ 🧪 vision-wk2	🧪 mac-wk2-stress	🛠 playstation
	✅ 🛠 tv	🧪 mac-intel-wk2
	✅ 🛠 tv-sim	✅ 🛠 mac-safer-cpp
	🛠 watch
	✅ 🛠 watch-sim

[webkit-commit-queue]

Committed 305809@main (df0854f): https://commits.webkit.org/305809@main

Reviewed commits have been landed. Closing PR #56710 and removing active labels.