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

WebGPURenderer default maxBufferSize too small #29865

Closed
AndrewChan2022 opened this issue Nov 12, 2024 · 4 comments
Closed

WebGPURenderer default maxBufferSize too small #29865

AndrewChan2022 opened this issue Nov 12, 2024 · 4 comments
Assignees
Labels
Milestone

Comments

@AndrewChan2022
Copy link

Description

WebGPURenderer backend default device.maxBufferSize = 256M too small, WebGLRenderer has no such limit.

However hardware limit is more than 256M, so user need explicitly get adapter.maxBufferSize to set param.requiredLimits.maxBufferSize.

Also check other limit such as maxStorageBufferBindingSize, ...

Solution

WebGPUBackend set necessary limit from adapter to device if user not set.

Alternatives

or a flag to require set limit from adapter to device.

Additional context

No response

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 19, 2024

WebGPU has the concept that when requesting a device, you get by default the minimum limits with no optional features so the app hopefully runs on all WebGPU supporting hardware (in the sense of "you developing the app with the minimum limits in mind").

It seems this issue is about always requesting the device limits. However, there are resources on the web which do not recommend this pattern (see https://webgpufundamentals.org/webgpu/lessons/webgpu-limits-and-features.html section "Don't request everything"). On the other hand, WebGPURenderer already request all supported features (see #25867 (comment) for the reason) so one could argue to do the same for limits.

The question is now does it make the developer's life easier if the engine always request the limits. To me, this would oppose the concept of WebGPU since the limits should enforce apps to think more about what resources they actually allocate. Since the developers already have the option to configure the limits, I vote to not change anything right now.

@mrdoob
Copy link
Owner

mrdoob commented Nov 20, 2024

Since the developers already have the option to configure the limits, I vote to not change anything right now.

Sounds good.

I also asked WebGPU developers and this is what they said:

I'd leave it up to the developer. Otherwise you may be lots of apps running on the developer's machine but not the customer's. Requiring the developer to ask is at least supposed to make them aware that they are choosing to have their app not run everywhere.

@Mugen87 Mugen87 closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2024
@Mugen87 Mugen87 added this to the r171 milestone Nov 20, 2024
@Spiri0
Copy link
Contributor

Spiri0 commented Dec 7, 2024

@AndrewChan2022 You can increase the limits yourself in three.weggpu.js if you absolutely need them. I tested this and it gave me a buffer size that was almost 8 times larger. I don't need that. I was just curious about the limits my hardware allows and in some parts these are significantly larger. But I don't dare assess how stable it is to push the limits beyond the set limits.

But if this is important to you, look for the device setup in three.webgpu.js and add the highlighted parts to it

let device;

if ( parameters.device === undefined ) {

	const adapterOptions = {
		powerPreference: parameters.powerPreference
	};

	const adapter = await navigator.gpu.requestAdapter( adapterOptions );

	if ( adapter === null ) {

		throw new Error( 'WebGPUBackend: Unable to create WebGPU adapter.' );

	}

	// feature support

	const features = Object.values( GPUFeatureName );

	const supportedFeatures = [];

	for ( const name of features ) {

		if ( adapter.features.has( name ) ) {

			supportedFeatures.push( name );

		}

	}

//---------------------add this to the device setup---------------------------
	const requiredLimits = {};

	for ( const key in adapter.limits ) {

		if ( key !== 'minSubgroupSize' && key !== 'maxSubgroupSize' ) {

			requiredLimits[ key ] = adapter.limits[ key ];

		}

	}
//----------------------------------------------------------------------------

	const deviceDescriptor = {
		requiredFeatures: supportedFeatures,
	//	requiredLimits: parameters.requiredLimits      //deactivate this line
		requiredLimits: requiredLimits    //    include this line
	};

	device = await adapter.requestDevice( deviceDescriptor );

	console.log(device);   // just for test to see the new limits

I don't need it, but would it perhaps even be something that could be offered as an option?
Something like enableMaximumAdapterLimits = true
what do the others think?

@AndrewChan2022
Copy link
Author

@Spiri0 Thanks a lot.

The framework also give opportunity to pass limit inside. This is my code.

const paras = {
    canvas: this.canvas,
    antialias: true,
    requiredLimits: {
        maxBufferSize: adapter.limits.maxBufferSize,
        maxStorageBufferBindingSize: adapter.limits.maxStorageBufferBindingSize,
    },
};
this.renderer = new THREERenderer(paras);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants