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

WebGPURenderer: Fix global references in Node.js #29919

Merged
merged 6 commits into from
Nov 24, 2024
Merged

WebGPURenderer: Fix global references in Node.js #29919

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df963e1
Fix compilation in react
whatisor Nov 19, 2024
1cb2183
Cleanup fixes
whatisor Nov 19, 2024
0b09367
Fix space
whatisor Nov 19, 2024
78800ae
Fix space
whatisor Nov 19, 2024
a32f487
Update WebGPUBackend.js
Mugen87 Nov 19, 2024
07c95d6
Update WebGPUUtils.js
Mugen87 Nov 24, 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
2 changes: 1 addition & 1 deletion src/renderers/webgpu/WebGPUBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class WebGPUBackend extends Backend {
powerPreference: parameters.powerPreference
};

const adapter = await navigator.gpu.requestAdapter( adapterOptions );
const adapter = ( typeof navigator !== 'undefined' ) ? await navigator.gpu.requestAdapter( adapterOptions ) : null;

if ( adapter === null ) {

Expand Down
6 changes: 3 additions & 3 deletions src/renderers/webgpu/nodes/WGSLNodeBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { GPUBufferBindingType, GPUStorageTextureAccess } from '../utils/WebGPUCo
import { NoColorSpace, FloatType } from '../../../constants.js';

// GPUShaderStage is not defined in browsers not supporting WebGPU
const GPUShaderStage = self.GPUShaderStage;
const GPUShaderStage = ( typeof self !== 'undefined' ) ? self.GPUShaderStage : { VERTEX: 1, FRAGMENT: 2, COMPUTE: 4 };

const gpuShaderStageLib = {
'vertex': GPUShaderStage ? GPUShaderStage.VERTEX : 1,
Expand Down Expand Up @@ -123,7 +123,7 @@ const wgslMethods = {

// WebGPU issue: does not support pow() with negative base on Windows

if ( /Windows/g.test( navigator.userAgent ) ) {
if ( typeof navigator !== 'undefined' && /Windows/g.test( navigator.userAgent ) ) {

wgslPolyfill.pow_float = new CodeNode( 'fn tsl_pow_float( a : f32, b : f32 ) -> f32 { return select( -pow( -a, b ), pow( a, b ), a > 0.0 ); }' );
wgslPolyfill.pow_vec2 = new CodeNode( 'fn tsl_pow_vec2( a : vec2f, b : vec2f ) -> vec2f { return vec2f( tsl_pow_float( a.x, b.x ), tsl_pow_float( a.y, b.y ) ); }', [ wgslPolyfill.pow_float ] );
Expand All @@ -141,7 +141,7 @@ if ( /Windows/g.test( navigator.userAgent ) ) {

let diagnostics = '';

if ( /Firefox|Deno/g.test( navigator.userAgent ) !== true ) {
if ( ( typeof navigator !== 'undefined' && /Firefox|Deno/g.test( navigator.userAgent ) ) !== true ) {

diagnostics += 'diagnostic( off, derivative_uniformity );\n';

Expand Down
14 changes: 11 additions & 3 deletions src/renderers/webgpu/utils/WebGPUUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,21 @@ class WebGPUUtils {
// TODO: Remove this check when Quest 34.5 is out
// https://github.com/mrdoob/three.js/pull/29221/files#r1731833949

if ( navigator.userAgent.includes( 'Quest' ) ) {
if ( typeof navigator !== 'undefined' ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove this? It is inside a function that will never be executed by the adapter return null in this case.

Copy link
Collaborator

@donmccurdy donmccurdy Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I guess the issue is that it's a compiler error, not a runtime error? I wish we knew what in the Next.js stack is doing compile-time checks on dependencies, and what its requirements are. Avoiding all references to browser globals is a pretty tall order for a large library relying heavily on Web APIs. :/

Copy link
Collaborator

Choose a reason for hiding this comment

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

The compiler throwing an error for a global variable inside a function seems strange to say the least, since a variable can be declared at the global level after the function declaration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I'm not sure why... trying to reproduce the issue:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is this one:
https://react.dev/reference/react/StrictMode


return GPUTextureFormat.BGRA8Unorm;
if ( navigator.userAgent.includes( 'Quest' ) ) {

return GPUTextureFormat.BGRA8Unorm;

} else {

return navigator.gpu.getPreferredCanvasFormat();

}

} else {

return navigator.gpu.getPreferredCanvasFormat();
return GPUTextureFormat.BGRA8Unorm;

}

Expand Down