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

Sroberge/2 0 6 #244

Merged
merged 7 commits into from
Oct 2, 2024
Merged

Sroberge/2 0 6 #244

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fb49110
Removed IObject, renamed Object to Obejct3D
vim-sroberge Sep 25, 2024
49cf3e4
Merge remote-tracking branch 'origin/main' into sroberge/2_0_6
vim-sroberge Sep 25, 2024
112b261
Merge remote-tracking branch 'origin/main' into sroberge/2_0_6
vim-sroberge Sep 25, 2024
2694758
"version": "2.0.6",
vim-sroberge Sep 25, 2024
1c35bed
fixed missing SelectableObject export
vim-sroberge Sep 25, 2024
0676026
Architectural' -> Object3D
vim-sroberge Oct 2, 2024
ad6235d
"version": "2.0.8",
vim-sroberge Oct 2, 2024
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
3 changes: 1 addition & 2 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ export * from './vim-loader/progressive/insertableMesh'
export * from './vim-loader/progressive/g3dSubset'
export * from './vim-loader/geometry'
export * from './vim-loader/materials/viewerMaterials'
export * from './vim-loader/object'
export * from './vim-loader/objectInterface'
export * from './vim-loader/object3D'
export * from './vim-loader/scene'
export * from './vim-loader/vim'
export * from './vim-loader/vimSettings'
Expand Down
5 changes: 2 additions & 3 deletions src/vim-loader/object.ts → src/vim-loader/object3D.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ import { IElement, VimHelpers } from 'vim-format'
import { ObjectAttribute } from './objectAttributes'
import { ColorAttribute } from './colorAttributes'
import { Submesh } from './mesh'
import { IObject, ObjectType } from './objectInterface'

/**
* High level api to interact with the loaded vim geometry and data.
*/
export class Object implements IObject {
export class Object3D {
private _color: THREE.Color | undefined
private _boundingBox: THREE.Box3 | undefined
private _meshes: Submesh[] | undefined
Expand All @@ -30,7 +29,7 @@ export class Object implements IObject {
/**
* Indicate whether this object is architectural or markup.
*/
public readonly type: ObjectType = 'Architectural'
public readonly type = 'Architectural'

/**
* The vim object from which this object came from.
Expand Down
94 changes: 0 additions & 94 deletions src/vim-loader/objectInterface.ts
View file
Open in desktop

This file was deleted.

26 changes: 13 additions & 13 deletions src/vim-loader/vim.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as THREE from 'three'
import { VimDocument, G3d, VimHeader, FilterMode } from 'vim-format'
import { Scene } from './scene'
import { VimSettings } from './vimSettings'
import { Object } from './object'
import { Object3D } from './object3D'
import {
ElementMapping,
ElementMapping2,
Expand Down Expand Up @@ -66,7 +66,7 @@ export class Vim {

private readonly _builder: SubsetBuilder
private readonly _loadedInstances = new Set<number>()
private readonly _elementToObject = new Map<number, Object>()
private readonly _elementToObject = new Map<number, Object3D>()

/**
* Getter for accessing the event dispatched whenever a subset begins or finishes loading.
Expand Down Expand Up @@ -158,15 +158,15 @@ export class Vim {
const elements = this.map.getElementsFromElementId(id)
return elements
?.map((e) => this.getObjectFromElement(e))
.filter((o): o is Object => o !== undefined) ?? []
.filter((o): o is Object3D => o !== undefined) ?? []
}

/**
* Retrieves the Vim object associated with the given Vim element index.
* @param {number} element - The index of the Vim element.
* @returns {Object | undefined} The Vim object corresponding to the element index, or undefined if not found.
* @returns {Object3D | undefined} The Vim object corresponding to the element index, or undefined if not found.
*/
getObjectFromElement (element: number): Object | undefined {
getObjectFromElement (element: number): Object3D | undefined {
if (!this.map.hasElement(element)) return

if (this._elementToObject.has(element)) {
Expand All @@ -176,18 +176,18 @@ export class Vim {
const instances = this.map.getInstancesFromElement(element)
const meshes = this.scene.getMeshesFromInstances(instances)

const result = new Object(this, element, instances, meshes)
const result = new Object3D(this, element, instances, meshes)
this._elementToObject.set(element, result)
return result
}

/**
* Retrieves an array containing all Vim objects strictly contained within the specified bounding box.
* @param {THREE.Box3} box - The bounding box to search within.
* @returns {Object[]} An array of Vim objects strictly contained within the bounding box.
* @returns {Object3D[]} An array of Vim objects strictly contained within the bounding box.
*/
getObjectsInBox (box: THREE.Box3) {
const result: Object[] = []
const result: Object3D[] = []

for (const obj of this.getObjects()) {
const b = obj.getBoundingBox()
Expand All @@ -201,10 +201,10 @@ export class Vim {

/**
* Retrieves an array of all objects within the Vim.
* @returns {Object[]} An array containing all objects within the Vim.
* @returns {Object3D[]} An array containing all objects within the Vim.
*/
getObjects () {
const result: Object[] = []
const result : Object3D[] = []
for (const e of this.map.getElements()) {
const obj = this.getObjectFromElement(e)
result.push(obj)
Expand All @@ -215,11 +215,11 @@ export class Vim {
/**
* Retrieves an array containing all objects within the specified subset.
* @param {G3dSubset} subset - The subset to retrieve objects from.
* @returns {Object[]} An array of objects within the specified subset.
* @returns {Object3D[]} An array of objects within the specified subset.
*/
getObjectsInSubset (subset: G3dSubset) {
const set = new Set<Object>()
const result: Object[] = []
const set = new Set<Object3D>()
const result: Object3D[] = []
const count = subset.getInstanceCount()
for (let i = 0; i < count; i++) {
const instance = subset.getVimInstance(i)
Expand Down
12 changes: 6 additions & 6 deletions src/vim-webgl-viewer/camera/cameraMovement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/

import { Camera } from './camera'
import { Object } from '../../vim-loader/object'
import { IObject } from '../../vim-loader/objectInterface'
import { Object3D } from '../../vim-loader/object3D'
import { SelectableObject } from '../../vim-webgl-viewer/selection'
import * as THREE from 'three'
import { GizmoMarker } from '../gizmos/markers/gizmoMarker'
import { Vim } from '../../vim-loader/vim'
Expand Down Expand Up @@ -119,9 +119,9 @@ export abstract class CameraMovement {

/**
* Rotates the camera without moving so that it looks at the specified target.
* @param {Object | THREE.Vector3} target - The target object or position to look at.
* @param {Object3D | THREE.Vector3} target - The target object or position to look at.
*/
abstract target(target: Object | THREE.Vector3): void
abstract target(target: Object3D | THREE.Vector3): void

/**
* Resets the camera to its last saved position and orientation.
Expand All @@ -141,10 +141,10 @@ export abstract class CameraMovement {
* @param {THREE.Vector3} [forward] - Optional forward direction after framing.
*/
frame (
target: IObject | Vim | THREE.Sphere | THREE.Box3 | 'all' | undefined,
target: SelectableObject | Vim | THREE.Sphere | THREE.Box3 | 'all' | undefined,
forward?: THREE.Vector3
): void {
if ((target instanceof GizmoMarker) || (target instanceof Object)) {
if ((target instanceof GizmoMarker) || (target instanceof Object3D)) {
target = target.getBoundingBox()
}
if ((target instanceof Vim)) {
Expand Down
6 changes: 3 additions & 3 deletions src/vim-webgl-viewer/camera/cameraMovementLerp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import * as THREE from 'three'
import { Camera } from './camera'
import { Object } from '../../vim-loader/object'
import { Object3D } from '../../vim-loader/object3D'
import { CameraMovementSnap } from './cameraMovementSnap'
import { CameraMovement } from './cameraMovement'

Expand Down Expand Up @@ -122,8 +122,8 @@ export class CameraLerp extends CameraMovement {
}
}

target (target: Object | THREE.Vector3): void {
const pos = target instanceof Object ? target.getCenter() : target
target (target: Object3D | THREE.Vector3): void {
const pos = target instanceof Object3D ? target.getCenter() : target
const next = pos.clone().sub(this._camera.position)
const start = this._camera.quaternion.clone()
const rot = new THREE.Quaternion().setFromUnitVectors(
Expand Down
6 changes: 3 additions & 3 deletions src/vim-webgl-viewer/camera/cameraMovementSnap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import { CameraMovement } from './cameraMovement'
import { Object } from '../../vim-loader/object'
import { Object3D } from '../../vim-loader/object3D'
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Issues Found: Missing or incorrect Object3D imports across multiple files.

Several files are using Object3D without proper import statements, which may lead to runtime errors or inconsistencies following the import path change in cameraMovementSnap.ts. Please update the import statements in the following files:

  • src/vim-webgl-viewer/selection.ts
  • src/vim-loader/object3D.ts
  • src/vim-loader/vim.ts
  • src/vim-loader/vim.ts
  • src/vim-loader/vim.ts
  • src/vim-loader/scene.ts
  • src/vim-webgl-viewer/gizmos/sectionBox/sectionBoxInputs.ts
  • docs/api/types/viw_webgl_viewer.ThreeIntersectionList.html
  • docs/api/classes/viw_webgl_viewer_rendering.RenderScene.html
  • docs/api/classes/viw_webgl_viewer_rendering.Renderer.html
  • docs/api/classes/viw_webgl_viewer_gizmos_sectionBox.BoxMesh.html
  • docs/api/classes/viw_webgl_viewer_gizmos_sectionBox.BoxInputs.html
  • docs/api/interfaces/vim_loader.IRenderer.html
  • src/vim-webgl-viewer/raycaster.ts
  • src/vim-webgl-viewer/environment.ts
  • src/vim-webgl-viewer/rendering/renderer.ts
  • src/vim-webgl-viewer/camera/cameraMovementLerp.ts
  • src/vim-webgl-viewer/rendering/renderScene.ts
  • src/vim-webgl-viewer/camera/cameraMovement.ts

Please ensure that each file properly imports Object3D from the correct path to maintain consistency across the project.

🔗 Analysis chain

LGTM! Verify consistent usage across the project.

The import statement for Object3D has been correctly updated. This change aligns with the method signature update in the target method.

To ensure consistency across the project, please run the following script to check for any other files that might need similar updates:

Adjusted Script for Verifying Object3D Usage

Please run the following script to ensure Object3D is consistently imported and used across the project:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other files that import Object3D and might need updates

# Search for files importing Object3D
echo "Files importing Object3D:"
rg --type typescript "import.*Object3D.*from" -g '!src/vim-webgl-viewer/camera/cameraMovementSnap.ts'

# Search for files using Object3D without an import (might be using a different import)
echo "\nFiles using Object3D without an import:"
rg --type typescript "Object3D" -g '!src/vim-webgl-viewer/camera/cameraMovementSnap.ts' | rg -v "import.*Object3D"

Length of output: 442

Script:

#!/bin/bash
# Description: Check for other files that import Object3D and might need updates

# Search for files importing Object3D without specifying the type
echo "Files importing Object3D:"
rg "import\s+.*Object3D\s+from\s+['\"]" --glob '!src/vim-webgl-viewer/camera/cameraMovementSnap.ts'

# Search for files using Object3D without an import
echo "\nFiles using Object3D without an import:"
rg "Object3D" --glob '!src/vim-webgl-viewer/camera/cameraMovementSnap.ts' | rg -v "import\s+.*Object3D"

Length of output: 114320

import * as THREE from 'three'

export class CameraMovementSnap extends CameraMovement {
Expand Down Expand Up @@ -43,8 +43,8 @@ export class CameraMovementSnap extends CameraMovement {
this.set(this._camera.position, target)
}

target (target: Object | THREE.Vector3): void {
const pos = target instanceof Object ? target.getCenter() : target
target (target: Object3D | THREE.Vector3): void {
const pos = target instanceof Object3D ? target.getCenter() : target
if (!pos) return
this.set(this._camera.position, pos)
}
Expand Down
38 changes: 15 additions & 23 deletions src/vim-webgl-viewer/gizmos/markers/gizmoMarker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { IElement } from 'vim-format'
import { ElementParameter } from 'vim-format/dist/vimHelpers'
import { Vim } from '../../../vim-loader/vim'
import { Viewer } from '../../viewer'
import * as THREE from 'three'
import { IObject, ObjectType } from '../../../vim-loader/objectInterface'
import { SimpleInstanceSubmesh } from '../../../vim-loader/mesh'
import { ObjectAttribute } from '../../../vim-loader/objectAttributes'
import { ColorAttribute } from '../../../vim-loader/colorAttributes'
Expand All @@ -12,8 +9,8 @@ import { ColorAttribute } from '../../../vim-loader/colorAttributes'
* Marker gizmo that display an interactive sphere at a 3D positions
* Marker gizmos are still under development.
*/
export class GizmoMarker implements IObject {
public readonly type: ObjectType = 'Marker'
export class GizmoMarker {
public readonly type = 'Marker'
private _viewer: Viewer
private _submesh: SimpleInstanceSubmesh

Expand Down Expand Up @@ -160,16 +157,21 @@ export class GizmoMarker implements IObject {
this._viewer.renderer.needsUpdate = true
}

getBimElement (): Promise<IElement> {
throw new Error('Method not implemented.')
get size () {
const matrix = new THREE.Matrix4()
this._submesh.mesh.getMatrixAt(this._submesh.index, matrix)
return matrix.elements[0]
Comment on lines +160 to +163
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 optimizing the size getter and clarify scaling assumptions

The current implementation of the size getter creates a new THREE.Matrix4 object on each call, which could be inefficient if called frequently. Consider caching this value if it's accessed often.

Also, the method only checks the first element of the matrix (index 0), assuming uniform scaling. If non-uniform scaling is possible or important for your use case, you might need to adjust this implementation.

Here's a potential optimization:

private _cachedSize: number | null = null;

get size(): number {
  if (this._cachedSize === null) {
    const matrix = new THREE.Matrix4();
    this._submesh.mesh.getMatrixAt(this._submesh.index, matrix);
    this._cachedSize = matrix.elements[0];
  }
  return this._cachedSize;
}

Don't forget to invalidate the _cachedSize when the size is changed.

}

getBimParameters (): Promise<ElementParameter[]> {
throw new Error('Method not implemented.')
}

get elementId (): any {
throw new Error('Method not implemented.')
set size (value: number) {
const matrix = new THREE.Matrix4()
this._submesh.mesh.getMatrixAt(this._submesh.index, matrix)
matrix.elements[0] = value
matrix.elements[5] = value
matrix.elements[10] = value
this._submesh.mesh.setMatrixAt(this._submesh.index, matrix)
this._submesh.mesh.instanceMatrix.needsUpdate = true
this._viewer.renderer.needsUpdate = true
Comment on lines +166 to +174
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 using THREE.js methods for matrix manipulation

The current implementation directly modifies matrix elements to set the size. While this is efficient, it might be more maintainable and less error-prone to use THREE.js built-in methods for matrix manipulation.

Also, the method enforces uniform scaling. If non-uniform scaling is a requirement for your use case, you might need to adjust this implementation.

Here's a suggestion using THREE.js methods:

set size(value: number) {
  const matrix = new THREE.Matrix4();
  this._submesh.mesh.getMatrixAt(this._submesh.index, matrix);
  const position = new THREE.Vector3();
  const quaternion = new THREE.Quaternion();
  const scale = new THREE.Vector3();
  matrix.decompose(position, quaternion, scale);
  scale.setScalar(value);
  matrix.compose(position, quaternion, scale);
  this._submesh.mesh.setMatrixAt(this._submesh.index, matrix);
  this._submesh.mesh.instanceMatrix.needsUpdate = true;
  this._viewer.renderer.needsUpdate = true;
  this._cachedSize = null; // Invalidate cache if you implement the optimization suggested for the getter
}

This approach uses decompose and compose methods, which might be more robust against future changes in the matrix structure.

}

/**
Expand All @@ -180,14 +182,4 @@ export class GizmoMarker implements IObject {
getBoundingBox (): THREE.Box3 {
return new THREE.Box3().setFromCenterAndSize(this.position.clone(), new THREE.Vector3(1, 1, 1))
}

/**
* Retrieves the center position of this object.
* @param {THREE.Vector3} [target=new THREE.Vector3()] Optional parameter specifying where to copy the center position data.
* A new instance is created if none is provided.
* @returns {THREE.Vector3 | undefined} The center position of the object.
*/
public getCenter (target?: THREE.Vector3): THREE.Vector3 {
return (target ?? new THREE.Vector3()).copy(this.position)
}
}
6 changes: 3 additions & 3 deletions src/vim-webgl-viewer/raycaster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import * as THREE from 'three'
import { Object } from '../vim-loader/object'
import { Object3D } from '../vim-loader/object3D'
import { Mesh } from '../vim-loader/mesh'
import { RenderScene } from './rendering/renderScene'
import { Viewport } from './viewport'
Expand All @@ -26,7 +26,7 @@ export type ActionModifier = 'none' | 'shift' | 'ctrl'
* Highlevel aggregate of information about a raycast result
*/
export class RaycastResult {
object: Object | GizmoMarker | undefined
object: Object3D | GizmoMarker | undefined
intersections: ThreeIntersectionList
firstHit: THREE.Intersection | undefined

Expand All @@ -46,7 +46,7 @@ export class RaycastResult {

private GetFirstVimHit (
intersections: ThreeIntersectionList
): [THREE.Intersection, Object] | [] {
): [THREE.Intersection, Object3D] | [] {
for (let i = 0; i < intersections.length; i++) {
const obj = this.getVimObjectFromHit(intersections[i])
if (obj?.visible) return [intersections[i], obj]
Expand Down
Loading