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.

Sign up for GitHub

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

Jump to bottom

Support for OffscreenCanvas in Web Workers #5168

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Support for OffscreenCanvas in Web Workers #5168

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0756ba8
Support for OffscreenCanvas in Web Workers #2481
michaelfranzl Mar 19, 2023
bb76873
fixup! Support for OffscreenCanvas in Web Workers #2481
michaelfranzl Mar 19, 2023
5c5db03
fixup! Support for OffscreenCanvas in Web Workers #2481
michaelfranzl Mar 21, 2023
7a7bbff
fixup! Support for OffscreenCanvas in Web Workers #2481
michaelfranzl Mar 21, 2023
c185bf4
fixup! Support for OffscreenCanvas in Web Workers #2481
michaelfranzl Mar 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"TWEEN": "readonly",
"twgsl": "readonly",
"webkitAudioContext": "readonly",
"WorkerGlobalScope": "readonly",
"XRRay": "readonly",
"XRWebGLLayer": "readonly"
},
Expand Down
11 changes: 9 additions & 2 deletions src/core/platform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ if (typeof navigator !== 'undefined') {
}

// detect browser/node environment
const environment = (typeof window !== 'undefined') ? 'browser' : 'node';
const environment = (typeof window !== 'undefined') ? 'browser' : (typeof WorkerGlobalScope !== 'undefined') ? 'webworker' : 'node';

/**
* Global namespace that stores flags regarding platform environment and features support.
Expand All @@ -78,7 +78,7 @@ const platform = {
*
* @type {object}
*/
global: (environment === 'browser') ? window : global,
global: (environment === 'browser') ? window : (environment === 'webworker' ? WorkerGlobalScope : global),

/**
* Convenience boolean indicating whether we're running in the browser.
Expand All @@ -87,6 +87,13 @@ const platform = {
*/
browser: environment === 'browser',

/**
* Convenience boolean indicating whether we're running in a web worker
willeastcott marked this conversation as resolved.
Show resolved Hide resolved
*
* @type {boolean}
*/
webworker: environment === 'webworker',

/**
* Is it a desktop or laptop device.
*
Expand Down
12 changes: 10 additions & 2 deletions src/framework/app-base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2152,7 +2152,10 @@ const makeTick = function (_app) {
setApplication(application);

if (frameRequest) {
window.cancelAnimationFrame(frameRequest);
if (platform.browser)
window.cancelAnimationFrame(frameRequest);
else if (platform.webworker)
cancelAnimationFrame(frameRequest);
frameRequest = null;
}

Expand All @@ -2171,7 +2174,12 @@ const makeTick = function (_app) {
if (application.xr?.session) {
frameRequest = application.xr.session.requestAnimationFrame(application.tick);
} else {
frameRequest = platform.browser ? window.requestAnimationFrame(application.tick) : null;
if (platform.browser)
frameRequest = platform.global.requestAnimationFrame(application.tick);
else if (platform.webworker)
frameRequest = requestAnimationFrame(application.tick);
else
frameRequest = null;
}

if (application.graphicsDevice.contextLost)
Expand Down
2 changes: 1 addition & 1 deletion src/framework/application.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class Application extends AppBase {
/**
* Create a new Application instance.
*
* @param {HTMLCanvasElement} canvas - The canvas element.
* @param {HTMLCanvasElement | OffscreenCanvas } canvas - The canvas element.
willeastcott marked this conversation as resolved.
Show resolved Hide resolved
* @param {object} [options] - The options object to configure the Application.
* @param {import('./input/element-input.js').ElementInput} [options.elementInput] - Input
* handler for {@link ElementComponent}s.
Expand Down
2 changes: 1 addition & 1 deletion src/platform/graphics/graphics-device.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ class GraphicsDevice extends EventHandler {
}

updateClientRect() {
this.clientRect = this.canvas.getBoundingClientRect();
this.clientRect = new DOMRect(0, 0, this.canvas.width, this.canvas.height);
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of issues here:

  1. DOMRect isn't available in Node.js so this causes the unit tests to fail.
  2. This function is called every frame so this will be generating garbage. Probably not a significant amount - just pointing it out.

Copy link
Author

Choose a reason for hiding this comment

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

I just pushed a commit with an alternative implementation which will generate no garbage and also works in Node.js: It's now a public class field (better for performance because "hidden class"). It's a simple object with properties width and height. From reviewing the code base, I can say that only width and height are used, and not x and y. The only reference is in src/framework/components/camera/component.js .

}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/platform/graphics/webgl/webgl-graphics-device.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,14 +815,14 @@ class WebglGraphicsDevice extends GraphicsDevice {

// #if _DEBUG
pushMarker(name) {
if (window.spector) {
if (platform.browser && window.spector) {
const label = DebugGraphics.toString();
window.spector.setMarker(`${label} #`);
}
}

popMarker() {
if (window.spector) {
if (platform.browser && window.spector) {
const label = DebugGraphics.toString();
if (label.length)
window.spector.setMarker(`${label} #`);
Expand Down