Skip to content
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

Move 3d rendering to a background web worker #2152

Merged
merged 57 commits into from
May 10, 2024
Merged

Move 3d rendering to a background web worker #2152

merged 57 commits into from
May 10, 2024

Conversation

Floppy
Copy link
Collaborator

@Floppy Floppy commented May 8, 2024

To improve performance, we are movign all the rendering into a web worker, as well as the loading.

This is pretty complicated and quite a big change...

Outstanding things todo:

  • fix camera target
  • handle keyboard control
  • check pan / zoom control
  • Fix 3MF loader needing lots of DOM objects unavailable in web worker
  • Refactor preview.ts to make sure it works with multiple on screen at once
  • Fix touch controls (preventDefault to avoid scrolling)
  • Restore click-to-load
  • Autoload if visible

Resolves #2071

@Floppy Floppy added the improvement Refactors and behind-the-scenes improvements label May 8, 2024
@Floppy Floppy marked this pull request as ready for review May 10, 2024 13:24
this.cbLoadProgress(percentage)
}

onLoad (model): void {
Copy link

Choose a reason for hiding this comment

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

Function onLoad has 67 lines of code (exceeds 25 allowed). Consider refactoring.

this.cbLoadProgress(percentage)
}

onLoad (model): void {
Copy link

Choose a reason for hiding this comment

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

Function onLoad has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.


const handler: Comlink.TransferHandler<Event, UnifiedEvent> = {
canHandle: (obj): obj is Event => (obj instanceof Event),
serialize: (ev: Event) => {
Copy link

Choose a reason for hiding this comment

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

Function serialize has 36 lines of code (exceeds 25 allowed). Consider refactoring.

this.scene.add(light2)
}

load (cbLoadComplete, cbLoadProgress, cbLoadError): void {
Copy link

Choose a reason for hiding this comment

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

Function load has 26 lines of code (exceeds 25 allowed). Consider refactoring.

this.container.clientWidth / this.container.clientHeight,
0.1,
1000
async run (): Promise<void> {
Copy link

Choose a reason for hiding this comment

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

Function run has 27 lines of code (exceeds 25 allowed). Consider refactoring.

@Floppy Floppy added the performance Speedy go fast label May 10, 2024
@Floppy Floppy enabled auto-merge May 10, 2024 13:32
@Floppy
Copy link
Collaborator Author

Floppy commented May 10, 2024

Touch controls still need testing, but the easiest way for me to test that is by deploying this to my own instance, so I'll check it after the merge.

Copy link

codeclimate bot commented May 10, 2024

Code Climate has analyzed commit 7a37d25 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 5

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 87.1% (0.0% change).

View more on Code Climate.

@Floppy Floppy merged commit 8df72f3 into main May 10, 2024
10 checks passed
@Floppy Floppy deleted the offscreencanvas branch May 10, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Refactors and behind-the-scenes improvements performance Speedy go fast
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Improve rendering performance with OffscreenCanvas
1 participant