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

feat: Add hidden raycasting check to <Html /> #341

Merged
merged 6 commits into from
May 19, 2021

Conversation

j-era
Copy link
Contributor

@j-era j-era commented Mar 24, 2021

Why

Check visibility by distance
Resolves #340

What

Add a 'checkDepth' property which can be set to true to activate testing if the HTML object is visible from the camera. Check is handled by raycasting.

Checklist

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

@vercel
Copy link

vercel bot commented Mar 24, 2021

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/B7Jt1hycZMcTvtqVtKvwZy369QDw
✅ Preview: https://drei-git-fork-j-era-feat-340-add-hidden-check-to-html-pmndrs.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 24, 2021

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 df905ea:

Sandbox Source
great-ritchie-84c4b Configuration
drei reflector Configuration
arc-x-pmndrs-colors Configuration

@gsimone
Copy link
Member

gsimone commented Mar 24, 2021

Hey 👋 did you see #337 ? I did the same thing but using an explicit occluder object to save on raycaster checks. What do you think?

@j-era
Copy link
Contributor Author

j-era commented Mar 24, 2021

Hi @gsimone, no unfortunately I didn't. The check I wrote tests if the position is visible in front of any geomtry hit in the scene. Attached to the same object it has to be positioned manually to ensure it is "outside" of the geometry.
But the option to limit the check to one (or more?) occluder object(s) and fading would be a good addition (instead of raycasting all scene children). Maybe those are individual features?

@gsimone
Copy link
Member

gsimone commented Mar 25, 2021

The "all scene" check could be implemented via the occluder API, like this

const { scene } = useThree()

<Html occluder={scene.children} /> 

The fading didn't really work so I'd probably just drop it, but I'd prefer defaulting to explicitely passing the object to , to avoid performance footguns.

I'll close #337 in favor of this, anyway ✅

@gsimone gsimone changed the title Add hidden check to html feat: Add hidden raycasting check to <Html /> Mar 25, 2021
@j-era
Copy link
Contributor Author

j-era commented Mar 25, 2021

Ok fine :) I'll update the implementation.

@joshuaellis joshuaellis marked this pull request as draft March 25, 2021 19:21
@drcmda
Copy link
Member

drcmda commented Mar 26, 2021

just like @gsimone's pr, having this in would be wonderful. most requested feature for html. occluder sounds pretty clever, but in whichever way - it could be finetuned little by little.

@drcmda
Copy link
Member

drcmda commented Mar 26, 2021

btw @gsimone @joshuaellis it would be pretty crazy if drei had a quick but not so precise bounds only raycaster. or something configurable.

import { Raycaster, aabb } from 'three-stdlib'

const ray = new RayCaster({ check: aabb }) // convex, vertex, etc ...
ray.intersect(...)

if we had this, r3f could use that by default. it could switch to low-res piercing on regress (movement) and things like <Html/> could pierce using the lowest setting. you think that would be possible?

The idea is that it could be modular (saving bundle size again). it wouldn't call mesh.raycast in that case but temporarily inject its own just like meshbounds does.

@joshuaellis
Copy link
Member

@gsimone i'm gonna leave this to you 👌🏼

@gsimone
Copy link
Member

gsimone commented Apr 1, 2021

I'll review as soon as possible 🙏

@joshuaellis
Copy link
Member

@j-era how can I test this?

@j-era
Copy link
Contributor Author

j-era commented May 18, 2021

sry @joshuaellis, no time for nothing ;) i've added a storybook example you can test with -> https://drei-git-fork-j-era-feat-340-add-hidden-check-to-html-pmndrs.vercel.app/?path=/story/misc-html--html-raycast-occluder-st

Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less code than I thought it would be, but looks like it works great 🙏🏼

@drcmda
Copy link
Member

drcmda commented May 18, 2021

nice! let's get it in if it works :-)

@joshuaellis joshuaellis merged commit 187c8d3 into pmndrs:master May 19, 2021
@github-actions
Copy link

🎉 This PR is included in version 5.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@drcmda
Copy link
Member

drcmda commented May 29, 2021

@j-era @joshuaellis Could we amend the docs a little? I think it's a little confusing, the story book mentions a occluder, and docs don't but do mention checkDepth instead. I think it's not entirely clear how this feature works and how it can be enabled.

looking into the code it looks like checkDepth is a left-over, it doesn't seem to do anything but that's the prop we documented.

@drcmda
Copy link
Member

drcmda commented May 29, 2021

made a small test here https://codesandbox.io/s/mixing-html-and-webgl-forked-hpqd3?file=/src/Html.tsx

it changes the prop to "occlude" and allows it to be a boolean. a true would occlude the whole scene. it also sets opacity instead of display, which would allow the user to animate it. i guess since we haven't documented or published the feature yet we could still just count it as a patch.

also added onOcclude so that the user can freely control/animate the transition. will make a new pr for this

@symbiosdotwiki
Copy link

Hey, This feature is really cool but it seems to drop the frame rate of DRASTICALLY when in use. Anyone else have this problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hidden check to <Html/>
5 participants