fix: pointer events offset for ScrollControls

[Glavin001]

Why

Fixes #600

What

Removed the raycaster.computeOffsets override. Not sure why it was added, however, appears to fix the problem.

Checklist

• Documentation updated
• Storybook entry added
• Ready to be merged

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/drei/5P9c4rURq663tMxkWMvsURERyGLS
✅ Preview: https://drei-git-fork-glavin001-fix-scrollcontrols-pointer-pmndrs.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1766ed2:

Sandbox	Source
sweet-khayyam-tnmc6	Configuration
Ground reflections and video textures	Configuration
arc-x-pmndrs-colors	Configuration
scrollcontrols-demo pointer events bug	Issue #600
react-three-fiber starter	Issue #600
scrollcontrols-demo pointer events bug	Issue #600

[Glavin001]

@joshuaellis (I see you have reviewed other PRs in the past): Please let me know if I can do anything to help the review and merge process. Thank you!

[drcmda]

imo this can't be the solution. clientX/Y should be right given that a scroll container is not scaled 100vw/vh but overlaps the canvas by n pages. without clientX/Y it would not be able to raycast any longer once scrollTop is > 0. generally, event handling on the dom is too complex, we've had this in the past - offsetXY and clientXY will always suit one use case but not the other. margins probably make it worse.

[drcmda]

btw the change for computeOffsets came from here: pmndrs/react-three-fiber#782

and he was suggesting

return {
offsetX: offsetY - state.scrollLeft,
offsetY: offsetY - state.scrollTop,
}

this could be fast and generic enough, because scrollcontrols stores scrollTop/Left, so it wouldn't cause layout thrashing if we fetch scrollleft/top in onScroll. i tried it out quickly with your box and it worked. so instead of removing compute alltogether maybe that could be a solution.

[Glavin001]

btw the change for computeOffsets came from here: pmndrs/react-three-fiber#782

and he was suggesting

return {
offsetX: offsetY - state.scrollLeft,
offsetY: offsetY - state.scrollTop,
}

I forked the related Codesandbox and was able to reproduce the problem:
❌ Using scrollTop / scrollLeft did not appear to work: https://codesandbox.io/s/r3flex-pointer-events-offset-for-scrollcontrols-pkgs5

I don't think using the scrollLeft / scrollTop solves the problem I'm trying to address.

I think we need to get the offset position of the canvas (i.e. gl.domElement.parentNode.offsetTop, etc).

i tried it out quickly with your box and it worked.

I may be misunderstanding what you're suggesting. If you have a patched Codesandbox could you share it?

What has worked for me so far has been using:

    const target = gl.domElement.parentNode
    return {
      offsetX: clientX - target.offsetLeft,
      offsetY: clientY - target.offsetTop
    }

I've pushed to this PR.

It fixes the issue in:
✅ My closed source project
✅ Codesandbox: https://codesandbox.io/s/scrollcontrols-demo-pointer-events-bug-b0b2w
✅ Storybook story with canvas being 100% viewport: https://drei-git-fork-glavin001-fix-scrollcontrols-pointer-pmndrs.vercel.app/?path=/story/controls-scrollcontrols--default-story
✅ Storybook story with margin/padding around canvas: https://drei-git-fork-glavin001-fix-scrollcontrols-pointer-pmndrs.vercel.app/?path=/story/controls-scrollcontrols--inside-container-story

Thanks for your help!

[joshuaellis]

I've had this problem before (if I understand correctly) and I think @Glavin001's solution should work correctly. From the links above this is also evident. @drcmda can you check those links and ensure you're happy with the resolution?

[drcmda]

it's very very strange. i still remember the github issues complaining about offsetXY and the pr for computeOffsets was made in order to allow them to exchange it in such cases for clientXY. though perhaps this was only relevant if the canvas itself is part of a scroll container. if we are sure that this doesn't break anything then im ok.