-
Notifications
You must be signed in to change notification settings - Fork 57
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
Pointer Events for mesh elements #254
base: develop
Are you sure you want to change the base?
Conversation
@keywizzle opened PR to track |
src/core/Scene.ts
Outdated
if (intersections.length == 0) return | ||
|
||
// Create custom event with detail of the exact XYZ position of the click | ||
const newEvent = new CustomEvent('click', { |
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 can use MouseEvent to emulate the real event and its properties https://developer.mozilla.org/en-US/docs/Web/API/Element/click_event
detail: {position: intersections[0].point}, | ||
}) | ||
// Dispatch custom event on the LUME element saved on the intersection Three.js object | ||
intersections[0].object.userData.lumeElement.dispatchEvent(newEvent) |
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 will need to check up the three.js tree because there can be three.js subnodes and we need to find under which lume element those subnodes are
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.
Not super familiar with the three.js tree, but I assume that not every three.js Object3d
will have a corresponding Lume element, so should it just check if lumeElement
exists on userData
, and if not, check the Object3d
's parent until one is found?
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.
Exactly, the element's .three
property will be a Three.js node, and it might have Three.js children. This upward check could go in getLumeElementFromThree
mentioned in the other comment.
src/core/ImperativeBase.ts
Outdated
@@ -182,6 +182,8 @@ export class ImperativeBase extends Settable(Transformable) { | |||
// Helpful for debugging when looking in devtools. | |||
// @prod-prune | |||
o.name = `${this.tagName}${this.id ? '#' + this.id : ''} (webgl, ${o.type})` | |||
// Save LUME element ref to Three.js object | |||
o.userData.lumeElement = this |
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.
For this we can use a Map so that it is typesafe. We'll need to eventually once we compile to WebAssembly anyway. We also need to unmap the pair on disconnect. If we use a weak map then we don't need to do the manual unmapping but assembly script doesn't support weak map yet, although we can implement it
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.
Yeah I think just a weak map would be good
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.
Another idea is we can make a subclass mix in or factory function that returns an instance with the lumeElement property
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.
Would the map be stored on the scene to keep track of each Object3d
's corresponding Lume element? Then when it's time to dispatch the event, the scene gets the object's element and dispatches it that way?
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.
It can just be in the top level of a module. We can export a function so that anyone can use it, for example:
import {getLumeElementFromThree} from 'lume'
getLumeElementFromThree(someThreejsObject)
and what this might do is traverse up until it finds a node associated with a Lume element using that map.
dc8cfa0
to
200cdbf
Compare
b5ca1cb
to
84505cd
Compare
-Dispatches custom event of type click on the first intersected object within a Scene
Co-authored-by: Kyle Bruce <keywizzle@users.noreply.github.com>
…so that we can stop the native events from happening on the CSS content without breaking devtools support, allowing only a single replicated event to be dispatched fot the 3D content. Also added pointermove, and copied all the needed properties from the native events to our synthetic events. Thanks helping think through this @keywizzle! Co-authored-by: Kyle Bruce <keywizzle@users.noreply.github.com>
…en and cyan planes, and on things blocking those planes, and it works as expected.
84505cd
to
526ff82
Compare
} | ||
|
||
return a | ||
} |
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.
- move to a utility file
@@ -206,6 +206,8 @@ export class SharedAPI extends DefaultBehaviors(ChildTracker(Settable(Transforma | |||
// Helpful for debugging when looking in devtools. | |||
// @prod-prune | |||
o.name = `${this.tagName}${this.id ? '#' + this.id : ''} (webgl, ${o.type})` | |||
// Save LUME element ref to Three.js object | |||
o.userData.lumeElement = this |
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.
- avoid using o.userData which has no types, we can use a
Map
orWeakMap
instead.
@@ -1189,6 +1380,8 @@ export class Scene extends SharedAPI { | |||
${super.css} | |||
|
|||
:host { | |||
pointer-events: auto; |
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 don't remember why we need to add this (or I forgot to remove it when experimenting). Let's add a comment, or remove if not needed.
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 saw this a while ago and don't know why. I know we messed with disabling pointer-events for CSS click events and maybe this was an artifact of that
@@ -19,161 +19,231 @@ | |||
|
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.
- Move this out of
test.html
into its own demo
@@ -685,6 +694,8 @@ export class Scene extends SharedAPI { | |||
override disconnectedCallback() { | |||
super.disconnectedCallback() | |||
this.#stopParentSizeObservation() | |||
this.removeEventListener('click', this.#handleClick) | |||
this.removeEventListener('pointermove', this.#handlePointermove) |
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 don't need to explicitly remove the handlers, as event an element with its own event listeners will be GC'd fine.
Question is, what's the most user-friendly way of defining specific event listeners of an instance? An easy method might be to add a method to instancedMesh.addInstanceEventListener(0, 'pointer-move', () => {
// Do whatever
} Maybe long term, it would be better to have an instancedMesh.instances[0].addEventListener('pointer-move', () => {
// Do whatever
} At that point, we might as well add instances to the DOM as a |
I think initially we can start with this: instancedMesh.addEventListener('pointermove', evt => {
evt.instanceId
} Where We could then provide simpler abstraction built on top of that. That's a really good idea about the mesh instance elements. Will greatly simplify the user experience. Example: <lume-instanced-mesh ...>
<lume-mesh-instance number="100" position=".." color=".." ...></lume-mesh-instance>
</lume-instanced-mesh> Where the child controls instance 100 specifically. And then: const instance = document.querySelector('lume-mesh-instance')
instance.addEventListener('pointermove', (evt) => {
instance.number === evt.instanceId // true
// ...handle pointer events only for this instance...
}) Naming TBD |
Some thoughts: For animations, the What other 3D details should be included the event? Currently it's only the world position, but three.js gives more details which we might want to include:
|
These are good thoughts, although we can save this for a separate change and focus this one on the pointer events. But yeah, we'd need to choose between emitting on the animation controller element, or on the model itself, or maybe even both. Anywho let's chat about that in a new issue.
That's true. I think first let's get basics working, then we can add more official event properties as needed in new PRs. We can dump any other properties in something like |
Closes #177
Currently only CSS-rendered elements emit events, based on their rectangle shapes.
Now we add
click
,pointermove
, etc, for mesh elements that aren't rectangles (using raycaster and manually emitting the events ourselves to emulate the same event as regular built-in elements).We will now be able to rely on shapes of meshes for all interactions!
'click'
'pointermove'
'pointerdown'
'pointerup'
'pointerenter'
'pointerleave'
'pointerover'
'pointerout'
'pointercancel'
?<lume-instanced-mesh>
per instance events'click'
'pointermove'
'pointerdown'
'pointerup'
'pointerenter'
'pointerleave'
'pointerover'
'pointerout'
'pointercancel'
?Eventful
. Some events have DOM equivalents.load
event for anything that loads resources, f.e.<lume-gltf-model>