<View /> Component assumes that the canvas is full screen

[kmannislands]

• three version: r139
• @react-three/fiber version: 8.x (any compatible with createPortal state "enclave")
• @react-three/drei version: 9.1.3 (any including View component)
• node version: 16
• npm (or yarn) version: 3.2.0

Problem description:

The nifty new View component seems to bake-in the assumption that the canvas is fullscreen. If the r3f canvas is not fullscreen, the Views within it are offset from the top/left by the same amount that the whole canvas is.

Relevant code:

I've modified one of the <View /> examples to demonstrate this.

https://codesandbox.io/s/view-skissor-forked-k46ley?file=/src/App.js

Suggested solution:

When calculating the scissor viewport in the <View /> component, the calculations for bottom and left should factor in the canvas element's own bounding rect as well as the "track" elements bounding rect.

Not sure what the best way to go about getting the canvas's bounding rect--it's no longer exposed on the state.size property like it used to be in older r3f versions (now it's just height, width. Used to include top and left).

[drcmda]

view bounds are being calculated here: https://github.com/pmndrs/drei/blob/master/src/web/View.tsx#L111 could you look into it? i have little understanding how it has to be done properly. @FarazzShaikh i think ... that instead of using getBoundingClientRect we should probably use this: https://github.com/pmndrs/react-use-measure it's the same lib that calculates r3f canvas bounds.

[FarazzShaikh]

Sure will look into it as well as the other issues with View

[FarazzShaikh]

@kmannislands To make sure I understand the issue correctly i have hard coded the fix, this is the expected result yes? and this the broken

In both demos, the red boxes are the track elemnts and the blue box is the canvas.

@drcmda we should expose the canvas bounds (useMeasure result) in RootState or in a new useCanvas hook since useMeasure requires the ref to be assigned to the canvas which is not directly accessible through View and it would be helpful to have. Perhaps we can add this to Krispy's hooks release.

[kmannislands]

@drcmda yep, I had a similar user land implementation (I called mine VirtualCanvas not View) that I'm in the process of updating from r3f v5 to latest so the difference was fairly obvious when state.size dropped left and bottom.

As a quick and very hacky/non-performant PoC, something like this works:

// Obviously bad:
let elem: HTMLElement | null = null;
function getCanvasBoundingRect(): DOMRect {
    if (!elem) {
        elem = document.querySelector('canvas');
    }

    return elem!.getBoundingClientRect();
}


// Computing correct positions for the View inside the use-frame:

// Includes information on the page position of the canvas, not just dimension:
const canvasSize = getCanvasBoundingRect();

const { left: trackElementLeft, right, top, bottom: trackElementBottom, width, height } = rect.current;
const left = trackElementLeft - canvasSize.left;
const bottom = canvasSize.bottom - trackElementBottom;

// ...

state.gl.setViewport(left, bottom, width, height);
state.gl.setScissor(left, bottom, width, height);

Happy to send a PR, just need some guidance on the best way to determine Canvas position

[kmannislands]

@FarazzShaikh yes, that's exactly what I was seeing. Thanks for the example there.

[FarazzShaikh]

Let’s just add back left and bottom to state.size. I’m not sure why it was dropped

[kmannislands]

I've taken a pass at adding top and left back to state.size. Here's an MR against my fork to have a look at the diff:

https://github.com/kmannislands/react-three-fiber/pull/1/files

@drcmda @FarazzShaikh is this a change that @react-three/fiber would consider?

It's an interface change in that state.size obviously changes and stat.setSize() has a different signature.

I adjusted setSize to take parameters in an order that I think makes the most sense but in the interest of better backwards compatibility, the parameters order could be changed. I think with that, the change could be considered entirely backwards compatible.

[FarazzShaikh]

Your PR is sane, can you PR it into r3f? Im unsure why the top and left were dropped, perhaps somone who worked on it at that time can shead some light there. Perhaps we can merge it keeping the signatrute of setSize non-breaking and then make it more sensable but breaking in v9.

Will keep this issue open and once these props are are added to r3f it is an easy solution here.

[kmannislands]

Okay, opened the PR in to r3f.

Assuming that is accepted and makes it in to the next release, what should the strategy be on my drei-side PR?

I can mark drei's peer requirement as requiring newer than the released version of r3f with top and left but that limits the compatibility of drei for users that may not use View or do not use View outside of full-screen.

Another approach that I find preferable would be to adapt the View component code to work either with or without top and left and to fall back on the old behavior if they are not present. Ideally, this would come with a logged dev mode warning.

[FarazzShaikh]

If it makes it into v8 then yes a fallback to current behavior with a console warning would be the way to go

If it makes it into v9 then we can bump peer deps.

Btw would you like me to assign this issue in Drei to you?

[kmannislands]

@FarazzShaikh makes sense. Yes sure

[Brandontam29]

Hello, is there a workaround for now? A project at my workplace depends on this. I would like to help if possible.

[CodyJasonBennett]

pmndrs/react-three-fiber#2339 was just shipped into 8.1.0.

[itsdouges]

Hey folks! Is the fix in https://github.com/pmndrs/react-three-fiber/pull/2339/files#diff-42e46ec762930e1485e849404cfda91d327e6ee4891ebbb7e9bc7c09f2cd01d0R285 meant to be checking clientTop? Is the border width the correct value to be looking at?

I would have imagined something similar to your original POC would have been needed? #944 (comment)

Alternatively exposing the canvas element in useThree could make it very easy for this to be fixed directly in drei without needing adjustments in r3f.

For context I had similar problems to this, where a scroll container (not body) would have the wrong position as it wasn't taking the scroll position of the container into account. I've fixed this for now by forking View and passing the canvas top data after calling getBoundingClientRect on it (similar to your POC).

[kmannislands]

Hey folks! Is the fix in https://github.com/pmndrs/react-three-fiber/pull/2339/files#diff-42e46ec762930e1485e849404cfda91d327e6ee4891ebbb7e9bc7c09f2cd01d0R285 meant to be checking clientTop? Is the border width the correct value to be looking at?

No indeed it is not! Good catch. I've raised a PR to fix that discrepancy: pmndrs/react-three-fiber#2406

Seems that this would only impact users of createRoot directly as opposed to <Canvas /> since the batteries-included component passes the correct Size from react-use-measure.

Agree that exposing the canvas element directly could be convenient. However, since <Canvas /> is implemented as a React.ForwardRef component, you can absolutely get at the element yourself. I could imagine providing it via context to avoid props drilling if it's needed.

[github-actions]

🎉 This issue has been resolved in version 9.42.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀