Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] marker occlusion #1579

Merged
merged 1 commit into from
Jul 17, 2024
Merged

[FIX] marker occlusion #1579

merged 1 commit into from
Jul 17, 2024

Conversation

Kurtil
Copy link
Contributor

@Kurtil Kurtil commented Jul 11, 2024

There is two parts here:

  • If I am correct, marker.worldPos = marker.origin + marker.rtcPos. Because the occlusion layers uses the marker.originin the shader, themarker.rtcPosshould be used to build positions instead ofmarker.worldPos`.

  • The commit XEOK-33 seems to introduce a bug in the occlusion tester because "OcclusionLayer markers' transformation is being applied through the OcclusionLayer::positions array" is not correct... ??

@Kurtil
Copy link
Contributor Author

Kurtil commented Jul 11, 2024

annotations_createWithMouse.html.zip

I've updated the annotations_createWithMouse to quickly test this PR.

Comment/uncomment the lines 70 and/or 93 to verify that it is working correctly :)

@paireks
Copy link
Member

paireks commented Jul 15, 2024

@MichalDybizbanskiCreoox could you please have a look there? :)

@MichalDybizbanskiCreoox
Copy link
Collaborator

Thank you @Kurtil for the PR.

The 1c06e71 commit rectifies an incorrect assignment in the OcclusionTester at 1c06e71#diff-b81e22131b6074c4e31ef6fd6e3cd96cb491eb46ea40497e3beb60b482e400d4
This fix was necessary for a scenario, in which an Annotation (which derives from a Marker) is created, but doesn't appear in the view if its worldPos is set immediately, like this:
annotations.createAnnotation({ occludable: true }).worldPos = [2,0,-1]

The example you provide would work before the fix because of the incorrectness being compensated by the original const worldPos = marker.worldPos; assignment in the OcclusionLayer.js at

const worldPos = marker.worldPos;

Your change of this original assignment to use marker.rtcPos seem to not reintroduce the older error (which is great), and at the same time add precision to the occlusion test being performed with the rtcPos property, covering your test case.

@xeolabs xeolabs merged commit 19529a6 into xeokit:master Jul 17, 2024
2 checks passed
@Kurtil Kurtil deleted the fix/markerPosition branch July 18, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants