Initial editor #49
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| dist | ||
| build | ||
| node_modules | ||
| .vscode/**/* | ||
| demo/assets/data/**/* | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| import * as THREE from 'three'; | ||
| import { getCurrentTime } from './Util.js'; | ||
|
|
||
| export class Editor { | ||
| constructor(viewer, options = {}) { | ||
| this.viewer = viewer; | ||
|
|
||
| this.editPanel = null; | ||
|
|
||
| this.pointerUpHandlerPlaneFinder = this.onPointerUpPlaneFinder.bind(this); | ||
|
|
||
| this.initialized = false; | ||
| this.init(); | ||
| } | ||
|
|
||
| init() { | ||
| if (this.initialized) return; | ||
|
|
||
| this.setupEditPanel(); | ||
|
|
||
| this.initialized = true; | ||
| } | ||
|
|
||
| setupEditPanel() { | ||
| this.editPanel = document.createElement('div'); | ||
| this.editPanel.style.position = 'absolute'; | ||
| this.editPanel.style.padding = '10px'; | ||
| this.editPanel.style.backgroundColor = '#cccccc'; | ||
| this.editPanel.style.border = '#aaaaaa 1px solid'; | ||
| this.editPanel.style.zIndex = 90; | ||
| this.editPanel.style.width = '375px'; | ||
| this.editPanel.style.fontFamily = 'arial'; | ||
| this.editPanel.style.fontSize = '10pt'; | ||
| this.editPanel.style.textAlign = 'left'; | ||
|
|
||
|
|
||
| const editTable = document.createElement('div'); | ||
| editTable.style.width = '100%'; | ||
|
|
||
|
|
||
| // Plane Finder | ||
| const planeFinder = document.createElement('div'); | ||
| planeFinder.style.width = '100%'; | ||
| planeFinder.style.display = 'flex'; | ||
| planeFinder.style.flexDirection = 'row'; | ||
| planeFinder.style.justifyContent = 'space-between'; | ||
|
|
||
| const planeFinderLabel = document.createElement('p'); | ||
| planeFinderLabel.id = 'planeFinderLabel'; | ||
| planeFinderLabel.innerHTML = | ||
|
Owner
There was a problem hiding this comment. I see this code in multiple places, maybe it's worth making an There was a problem hiding this comment. Yes, there are a few places where we could refactor to separate functions |
||
| `Up: ${this.viewer.camera.up.x.toFixed(3)}, ${this.viewer.camera.up.y.toFixed(3)}, ${this.viewer.camera.up.z.toFixed(3)}`; | ||
|
|
||
| const planeFinderButton = document.createElement('button'); | ||
| planeFinderButton.innerHTML = 'Find ground plane'; | ||
| planeFinderButton.addEventListener('click', () => { | ||
| this.setupPlaneFinder(); | ||
| }); | ||
|
|
||
| planeFinder.appendChild(planeFinderLabel); | ||
| planeFinder.appendChild(planeFinderButton); | ||
|
|
||
| editTable.appendChild(planeFinder); | ||
|
|
||
| this.editPanel.appendChild(editTable); | ||
| this.editPanel.style.display = 'block'; | ||
| this.viewer.renderer.domElement.parentElement.prepend(this.editPanel); | ||
| } | ||
|
|
||
| setupPlaneFinder() { | ||
| if (this.viewer.useBuiltInControls) { | ||
| this.viewer.rootElement.removeEventListener('pointerup', this.viewer.pointerUpHandler); | ||
|
Owner
There was a problem hiding this comment. I definitely think we need a less intrusive way of overriding behavior in There was a problem hiding this comment. I'm mainly a React developer and am not all that familiar with best practices in vanilla and OOP.This could be a case of lack of knowledge on my part. The effect I was going for was to have a default global state that is the same as just the viewer. I couldn't really figure out how to do it with inheritance + overriding without loosing the default state. In my mind we had to store those handlers away anyhow and I figured that we might aswell keep the class itself intact as that stored default state.
Owner
There was a problem hiding this comment. So I was thinking something along the lines of: class Viewer {
constructor() {
window.addEventListener("mousedown", this.onMouseDown.bind(this));
}
onMouseDown() {
// default behavior
}
}
const PointerMode = {
Default: 0,
CameraUpSelection: 1
};
class Editor extends Viewer {
constructor() {
super();
this.pointerMode = PointerMode.Default;
this.setupEditPanel();
}
setupEditPanel() {
const planeFinderButton = document.createElement('button');
planeFinderButton.innerHTML = 'Find ground plane';
planeFinderButton.addEventListener('click', () => {
this.pointerMode = PointerMode.CameraUpSelection;
});
}
// override Viewer.onMouseDown()
onMouseDown() {
if (this.pointerMode === PointerMode.CameraUpSelection) {
// Do custom stuff for camera up selection
} else {
super.onMouseDown();
}
}
}This would retain default behavior when not selecting the camera-up vector. (This code probably contains errors, but hopefully it conveys the idea :) ) There was a problem hiding this comment. Yes, this is absolutely more clear, I like it. With this structure I would prefer to inherit the Viewer class. I also think a user should be able to jack into the events and extend the functionality in a straight forward manner. I'm not really sure how one would do that right now, maybe something like how I bound the invalidate() method to the controllers change event. Is there something with this approach that would hinder that?
Owner
There was a problem hiding this comment. I don't think an inheritance based approach will hinder that; we can still extend functions in There was a problem hiding this comment. +1 on making |
||
| this.viewer.rootElement.addEventListener('pointerup', this.pointerUpHandlerPlaneFinder); | ||
| } | ||
| } | ||
|
|
||
| teardownPlaneFinder() { | ||
| if (this.viewer.useBuiltInControls) { | ||
| this.viewer.rootElement.removeEventListener('pointerup', this.pointerUpHandlerPlaneFinder); | ||
| this.viewer.rootElement.addEventListener('pointerup', this.viewer.pointerUpHandler); | ||
| } | ||
| } | ||
|
|
||
| onPointerUpPlaneFinder = function() { | ||
| const renderDimensions = new THREE.Vector2(); | ||
| const clickOffset = new THREE.Vector2(); | ||
| // const toNewFocalPoint = new THREE.Vector3(); | ||
| const outHits = []; | ||
| let points = []; | ||
|
|
||
| return function(mouse) { | ||
| clickOffset.copy(this.viewer.mousePosition).sub(this.viewer.mouseDownPosition); | ||
| const mouseUpTime = getCurrentTime(); | ||
| const wasClick = mouseUpTime - this.viewer.mouseDownTime < 0.5 && clickOffset.length() < 2; | ||
|
Owner
There was a problem hiding this comment. I think we need to change |
||
|
|
||
| if (!this.transitioningCameraTarget && wasClick) { | ||
| this.viewer.getRenderDimensions(renderDimensions); | ||
| outHits.length = 0; | ||
| this.viewer.raycaster.setFromCameraAndScreenPosition(this.viewer.camera, this.viewer.mousePosition, renderDimensions); | ||
| this.viewer.mousePosition.set(mouse.offsetX, mouse.offsetY); | ||
| this.viewer.raycaster.intersectSplatMesh(this.viewer.splatMesh, outHits); | ||
|
|
||
| if (outHits.length > 0) { | ||
| const intersectionPoint = outHits[0].origin; | ||
|
|
||
| points.push(intersectionPoint); | ||
|
|
||
| if (points.length === 3) { | ||
| const plane = new THREE.Plane(); | ||
|
|
||
| plane.setFromCoplanarPoints(points[0], points[1], points[2]); | ||
|
|
||
| this.viewer.camera.up = plane.normal; | ||
| this.viewer.invalidate(); | ||
|
|
||
| // Update label | ||
| const planeFinderLabel = document.getElementById('planeFinderLabel'); | ||
| planeFinderLabel.innerHTML = | ||
| `${this.viewer.camera.up.x.toFixed(3)},${this.viewer.camera.up.y.toFixed(3)},${this.viewer.camera.up.z.toFixed(3)}`; | ||
|
|
||
| points = []; | ||
| this.teardownPlaneFinder(); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| }(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| import { Editor } from './Editor.js'; | ||
| import { PlyLoader } from './PlyLoader.js'; | ||
| import { PlyParser } from './PlyParser.js'; | ||
| import { SplatBuffer } from './SplatBuffer.js'; | ||
| import { SplatLoader } from './SplatLoader.js'; | ||
| import { Viewer } from './Viewer.js'; | ||
|
|
||
| export { | ||
| PlyParser, | ||
| PlyLoader, | ||
| SplatLoader, | ||
| SplatBuffer, | ||
| Viewer, | ||
| Editor, | ||
| }; |
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.
Do we need to add this? The
demodirectory is really meant to be treated as a source code directory (which means maybe it should actually be moved under/src). Would it be possible to usebuild/demo/assets/datafor the same purpose?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.
No, not really, I just didn't want to accidentally commit unnecessary files.
Not sure if it's appropriate with the
build/demo/assets/data-route. I would say it depends on what the goal/vision is, which I can't say I see clearly.