-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
improve raycaster component (fixes #829) #1196
Conversation
Cool, I was also thinking it would be useful to get the full intersection data in events. I got it working in my cursor-hitpoint branch, with an example: amberroy@593ab4c |
Yeah, returning full intersection data is part of the changes in that huge list. |
<a-mixin id="green" material="color: green"></a-mixin> | ||
<a-mixin id="blue" material="color: blue"></a-mixin> | ||
<a-mixin id="yellow" material="color: yellow"></a-mixin> | ||
<a-mixin id="cube-hovered" material="color: #CC435F"></a-mixin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you replacing color names with hex values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Color palette.
e35f581
to
73864d5
Compare
64d2bbe
to
6d18e04
Compare
7ec81cc
to
25edaac
Compare
616841e
to
095fe62
Compare
</a-assets> | ||
|
||
<!-- Camera + Cursor. --> | ||
<a-entity camera look-controls> | ||
<a-entity mixin="cursor" position="0 0 -3"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not be using primitives here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is gonna be the boilerplate. <a-cursor>
is one of the times i prefer the primitive because manually configuring the inner/outer radius of the ring is hard.
// Update matrix world. | ||
object3D.updateMatrixWorld(); | ||
// Grab the position and rotation. | ||
object3D.matrixWorld.decompose(originVec3, directionHelper, scaleDummy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still make directionHelper
, originVec3
and scaleDummy
private to this function so no other method relies on it to store any state. Like in this example:
https://github.com/aframevr/aframe/blob/master/src/components/look-controls.js#L111
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should wrap directionHelper, originVec3 and scaleDummy in a closure of the method so they're not allocated on every frame. Like the example I had pointed out:
https://github.com/aframevr/aframe/blob/master/src/components/look-controls.js#L111
// Update matrix world. | ||
object3D.updateMatrixWorld(); | ||
// Grab the position and rotation. | ||
object3D.matrixWorld.decompose(originVec3, directionHelper, scaleDummy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add scaleDummy to the closure as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not actually using scaleDummy
. I just pass it because decompose
will expect it.
Description: Improve overall cursor + raycaster components with respect to performance, configuration, and code quality. This is a stepping stone for more complex cursor/interactions later on.
DEMO
Changes proposed:
cursor-
/raycaster-
)cursor.maxDistance
as it was redundant withraycaster.far