web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
5k stars 3.09k forks source link

[hit-test][webxr] Tests seem to be missing some intersections #22898

Open Manishearth opened 4 years ago

Manishearth commented 4 years ago

https://github.com/web-platform-tests/wpt/blob/f2e918abb4b8711afb28c8b98d98be3bffc9fa9a/webxr/hit-test/ar_hittest_subscription_refSpaces.https.html#L110-L130

In this test, the viewer is at (0, 1, 0) and looking straight ahead (the default XRRay). This should intersect with both of the faces in front of the viewer since height is 2.0 and y=1 is where their midpoint is.

cc @bialpio

bialpio commented 4 years ago

Intersection with both of the faces results in exactly the same pose, so in Chrome's mock implementation I am deduplicating those kinds of results. Both faces that intersect the ray belong to the same mesh that is supposed to represent a single, contiguous surface, so it made no sense to me to return 2 results in this case. Let me know if you think we should revisit this approach.

Manishearth commented 4 years ago

This isn't mentioned in the spec anywhere. I don't think it's a good idea to make the test dependent on such an implementation detail, perhaps the ray should be shifted up a little bit.

Manishearth commented 4 years ago

@bialpio do you agree with shifting the ray up a little bit? if not this needs to be a spec bug

Manishearth commented 4 years ago

We can also tweak the test to allow both one and two hit test results, and then tweak the spec to allow this optimization.

bialpio commented 4 years ago

Shifting it up SGTM, as long as it makes the manual calculations still feasible - I usually calculate the expected results by hand before codifying them in order to avoid setting the expected data to whatever makes the tests pass.

If we were to spec it out, some questions we'd have to answer:

I think it's probably OK to dodge those for now and move the ray or the space relative to which it's expressed.

Manishearth commented 4 years ago

Yeah, most of the vectors are along the axes so shifting up is easy.

If we were to spec it out I'd imagine that we just spec that we filter out duplicates.

I'll fix this once https://github.com/web-platform-tests/wpt/pull/23350 lands to avoid test conflicts.