Skip to content
This repository was archived by the owner on Dec 17, 2024. It is now read-only.

Plan2D #247

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"@types/three": "^0.143.0",
"deepmerge": "^4.2.2",
"is-plain-object": "^5.0.0",
"pdfjs-dist": "^4.6.82",
"ste-events": "^3.0.7",
"ste-signals": "^3.0.9",
"ste-simple-events": "^3.0.7",
Expand Down
7 changes: 6 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Viewer, open, THREE, getViewerSettingsFromUrl } from '.'
import { Viewer, open, getViewerSettingsFromUrl } from '.'
import * as THREE from 'three'

// Parse URL for source file
const params = new URLSearchParams(window.location.search)
Expand Down Expand Up @@ -36,6 +37,10 @@ async function load (url: string | ArrayBuffer) {
viewer.camera.save()
viewer.gizmos.loading.visible = false

const plan = await viewer.gizmos.plans.addPlan('https://vimdevelopment01storage.blob.core.windows.net/samples/floor_plan.png')
const plan2 = await viewer.gizmos.plans.addPlan('https://vimdevelopment01storage.blob.core.windows.net/samples/floor_plan.pdf')
plan.color = new THREE.Color(0x00ff00)

Comment on lines +40 to +43
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider parameterizing plan URLs and colors

The addition of plans to the viewer's gizmos enhances its capabilities. However, there are a few points to consider:

  1. The URLs for the plans are hardcoded. Consider parameterizing these for better maintainability.
  2. Only the first plan's color is set. Is this intentional? If not, consider setting colors for both plans or using a consistent approach.

Here's a suggestion to improve this section:

const planUrls = [
  'https://vimdevelopment01storage.blob.core.windows.net/samples/floor_plan.png',
  'https://vimdevelopment01storage.blob.core.windows.net/samples/floor_plan.pdf'
];

const planColors = [
  new THREE.Color(0x00ff00),
  new THREE.Color(0x0000ff)  // Example second color
];

for (let i = 0; i < planUrls.length; i++) {
  const plan = await viewer.gizmos.plans.addPlan(planUrls[i]);
  plan.color = planColors[i];
}

This approach allows for easier management of multiple plans and their colors.

// Useful for debuging in console.
globalThis.THREE = THREE
globalThis.vim = vim
Expand Down
5 changes: 4 additions & 1 deletion src/vim-loader/colorAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import * as THREE from 'three'
import { MergedSubmesh } from './mesh'
import { MergedSubmesh, SimpleMesh } from './mesh'
import { Vim } from './vim'
import { InsertableSubmesh } from './progressive/insertableSubmesh'
import { AttributeTarget } from './objectAttributes'
Expand Down Expand Up @@ -101,6 +101,9 @@ export class ColorAttribute {
this.resetMergedInsertableColor(sub)
return
}
if (sub instanceof SimpleMesh) {
return
}

const colors = sub.three.geometry.getAttribute(
'color'
Expand Down
25 changes: 20 additions & 5 deletions src/vim-loader/mesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export class Mesh {
* Returns submesh for given index.
*/
getSubMesh (index: number) {
return new StandardSubmesh(this, index)
return new SealedSubmesh(this, index)
}

/**
Expand All @@ -126,15 +126,15 @@ export class Mesh {
throw new Error('Can only be called when mesh.merged = true')
}
const index = this.binarySearch(this.submeshes, faceIndex * 3)
return new StandardSubmesh(this, index)
return new SealedSubmesh(this, index)
}

/**
*
* @returns Returns all submeshes
*/
getSubmeshes () {
return this.instances.map((s, i) => new StandardSubmesh(this, i))
return this.instances.map((s, i) => new SealedSubmesh(this, i))
}

private binarySearch (array: number[], element: number) {
Expand Down Expand Up @@ -164,7 +164,8 @@ export class Mesh {
}

// eslint-disable-next-line no-use-before-define
export type MergedSubmesh = StandardSubmesh | InsertableSubmesh
export type MergedSubmesh = SealedSubmesh | InsertableSubmesh | SimpleMesh
// eslint-disable-next-line no-use-before-define
export type Submesh = MergedSubmesh | InstancedSubmesh

export class SimpleInstanceSubmesh {
Expand All @@ -179,7 +180,21 @@ export class SimpleInstanceSubmesh {
}
}

export class StandardSubmesh {
export class SimpleMesh {
mesh: THREE.Mesh
get three () { return this.mesh }
readonly index : number = 0
readonly merged = true
readonly meshStart = 0
readonly meshEnd : number

constructor (mesh: THREE.Mesh) {
this.mesh = mesh
this.meshEnd = mesh.geometry.index!.count
}
}
Comment on lines +183 to +195
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential null reference of 'mesh.geometry.index'

In the SimpleMesh constructor, you use mesh.geometry.index!.count with a non-null assertion. If mesh.geometry.index is null or undefined, this will cause a runtime error. Please ensure that mesh.geometry.index is always defined or add a null check to handle cases where it is not.

Consider modifying the constructor to handle undefined mesh.geometry.index:

constructor (mesh: THREE.Mesh) {
  this.mesh = mesh
- this.meshEnd = mesh.geometry.index!.count
+ if (mesh.geometry.index) {
+   this.meshEnd = mesh.geometry.index.count
+ } else {
+   this.meshEnd = mesh.geometry.attributes.position.count / mesh.geometry.attributes.position.itemSize
+ }
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export class SimpleMesh {
mesh: THREE.Mesh
get three () { return this.mesh }
readonly index : number = 0
readonly merged = true
readonly meshStart = 0
readonly meshEnd : number
constructor (mesh: THREE.Mesh) {
this.mesh = mesh
this.meshEnd = mesh.geometry.index!.count
}
}
export class SimpleMesh {
mesh: THREE.Mesh
get three () { return this.mesh }
readonly index : number = 0
readonly merged = true
readonly meshStart = 0
readonly meshEnd : number
constructor (mesh: THREE.Mesh) {
this.mesh = mesh
if (mesh.geometry.index) {
this.meshEnd = mesh.geometry.index.count
} else {
this.meshEnd = mesh.geometry.attributes.position.count / mesh.geometry.attributes.position.itemSize
}
}
}


export class SealedSubmesh {
mesh: Mesh
index: number

Expand Down
6 changes: 3 additions & 3 deletions src/vim-loader/objectAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
*/

import * as THREE from 'three'
import { MergedSubmesh, SimpleInstanceSubmesh, Submesh } from './mesh'
import { MergedSubmesh, SimpleInstanceSubmesh, SimpleMesh, Submesh } from './mesh'

export type AttributeTarget = Submesh | SimpleInstanceSubmesh
export type AttributeTarget = Submesh | SimpleInstanceSubmesh | SimpleMesh

export class ObjectAttribute<T> {
readonly vertexAttribute: string
Expand Down Expand Up @@ -51,7 +51,7 @@ export class ObjectAttribute<T> {
for (let m = 0; m < this._meshes.length; m++) {
const sub = this._meshes[m]
if (sub.merged) {
this.applyMerged(sub as MergedSubmesh, number)
this.applyMerged(sub, number)
} else {
this.applyInstanced(sub, number)
}
Expand Down
8 changes: 1 addition & 7 deletions src/vim-loader/progressive/insertableSubmesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export class InsertableSubmesh {
mesh: InsertableMesh
index: number
private _colors: Float32Array
readonly merged = true

constructor (mesh: InsertableMesh, index: number) {
this.mesh = mesh
Expand All @@ -26,13 +27,6 @@ export class InsertableSubmesh {
return this.mesh.mesh
}

/**
* True if parent mesh is merged.
*/
get merged () {
return true
}

private get submesh () {
return this.mesh.geometry.submeshes[this.index]
}
Expand Down
8 changes: 1 addition & 7 deletions src/vim-loader/progressive/instancedSubmesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { InstancedMesh } from './instancedMesh'
export class InstancedSubmesh {
mesh: InstancedMesh
index: number
readonly merged = false

constructor (mesh: InstancedMesh, index: number) {
this.mesh = mesh
Expand All @@ -25,13 +26,6 @@ export class InstancedSubmesh {
return this.mesh.mesh
}

/**
* True if parent mesh is merged.
*/
get merged () {
return false
}

/**
* Returns vim instance associated with this submesh.
*/
Expand Down
26 changes: 24 additions & 2 deletions src/vim-webgl-viewer/camera/camera.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ export interface ICamera {
*/
stop () : void

/**
* Immediately stops the camera movement. And prevents any further movement until set to false.
*/
freeze: boolean

/**
* The target at which the camera is looking at and around which it rotates.
*/
Expand Down Expand Up @@ -152,6 +157,7 @@ export class Camera implements ICamera {
private _inputVelocity = new THREE.Vector3()
private _velocity = new THREE.Vector3()
private _speed: number = 0
private _freeze: boolean = false

// orbit
private _orthographic: boolean = false
Expand Down Expand Up @@ -204,13 +210,25 @@ export class Camera implements ICamera {
/** Ignore movement permissions when true */
private _force: boolean = false

get freeze () {
return this._freeze
}

set freeze (value: boolean) {
this._freeze = value
}

/**
* Represents allowed movement along each axis using a Vector3 object.
* Each component of the Vector3 should be either 0 or 1 to enable/disable movement along the corresponding axis.
*/
private _allowedMovement = new THREE.Vector3(1, 1, 1)
get allowedMovement () {
return this._force ? new THREE.Vector3(1, 1, 1) : this._allowedMovement
return this._force
? new THREE.Vector3(1, 1, 1)
: this._freeze
? new THREE.Vector3(0, 0, 0)
: this._allowedMovement
Comment on lines +227 to +231
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve readability by replacing nested ternary operators with if-else statements

Nested ternary operators can be hard to read and maintain. Consider refactoring the getter to use if-else statements for better clarity.

Apply this diff to improve readability:

  get allowedMovement () {
-    return this._force
-      ? new THREE.Vector3(1, 1, 1)
-      : this._freeze
-        ? new THREE.Vector3(0, 0, 0)
-        : this._allowedMovement
+    if (this._force) {
+      return new THREE.Vector3(1, 1, 1)
+    } else if (this._freeze) {
+      return new THREE.Vector3(0, 0, 0)
+    } else {
+      return this._allowedMovement
+    }
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return this._force
? new THREE.Vector3(1, 1, 1)
: this._freeze
? new THREE.Vector3(0, 0, 0)
: this._allowedMovement
get allowedMovement () {
if (this._force) {
return new THREE.Vector3(1, 1, 1)
} else if (this._freeze) {
return new THREE.Vector3(0, 0, 0)
} else {
return this._allowedMovement
}
}

}

set allowedMovement (axes: THREE.Vector3) {
Expand All @@ -225,7 +243,11 @@ export class Camera implements ICamera {
* Each component of the Vector2 should be either 0 or 1 to enable/disable rotation around the corresponding axis.
*/
get allowedRotation () {
return this._force ? new THREE.Vector2(1, 1) : this._allowedRotation
return this._force
? new THREE.Vector2(1, 1)
: this._freeze
? new THREE.Vector2(0, 0)
: this._allowedRotation
Comment on lines +246 to +250
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve readability by replacing nested ternary operators with if-else statements

Nested ternary operators can be hard to read and maintain. Consider refactoring the getter to use if-else statements for better clarity.

Apply this diff to improve readability:

  get allowedRotation () {
-    return this._force
-      ? new THREE.Vector2(1, 1)
-      : this._freeze
-        ? new THREE.Vector2(0, 0)
-        : this._allowedRotation
+    if (this._force) {
+      return new THREE.Vector2(1, 1)
+    } else if (this._freeze) {
+      return new THREE.Vector2(0, 0)
+    } else {
+      return this._allowedRotation
+    }
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return this._force
? new THREE.Vector2(1, 1)
: this._freeze
? new THREE.Vector2(0, 0)
: this._allowedRotation
get allowedRotation () {
if (this._force) {
return new THREE.Vector2(1, 1)
} else if (this._freeze) {
return new THREE.Vector2(0, 0)
} else {
return this._allowedRotation
}
}

}

set allowedRotation (axes: THREE.Vector2) {
Expand Down
4 changes: 4 additions & 0 deletions src/vim-webgl-viewer/gizmos/gizmos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { IMeasure, Measure } from './measure/measure'
import { SectionBox } from './sectionBox/sectionBox'
import { GizmoMarkers } from './markers/gizmoMarkers'
import { Camera } from '../camera/camera'
import { Plans2D } from './plans2D'

/**
* Represents a collection of gizmos used for various visualization and interaction purposes within the viewer.
Expand Down Expand Up @@ -53,6 +54,8 @@ export class Gizmos {
*/
readonly markers: GizmoMarkers

readonly plans : Plans2D

constructor (viewer: Viewer, camera : Camera) {
this.viewer = viewer
this._measure = new Measure(viewer)
Expand All @@ -67,6 +70,7 @@ export class Gizmos {
this.rectangle = new GizmoRectangle(viewer)
this.axes = new GizmoAxes(camera, viewer.viewport, viewer.settings.axes)
this.markers = new GizmoMarkers(viewer)
this.plans = new Plans2D(viewer)
viewer.viewport.canvas.parentElement?.prepend(this.axes.canvas)
}

Expand Down
Loading