Skip to content

WebGPURenderer: Remove _forceViewport #31189

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

Merged
merged 1 commit into from
May 28, 2025
Merged

Conversation

cabanier
Copy link
Contributor

This addresses review #31134 (comment)

@Mugen87 , does this diff implement what you were looking for in #31134 (comment)? Or there calls to setSize I can remove?

Copy link

github-actions bot commented May 28, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 337.26
78.64
337.26
78.64
+0 B
+0 B
WebGPU 552.95
153.31
552.92
153.31
-29 B
-5 B
WebGPU Nodes 552.3
153.16
552.27
153.15
-29 B
-6 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 468.44
113.28
468.44
113.28
+0 B
+0 B
WebGPU 627.97
170
627.94
169.98
-28 B
-14 B
WebGPU Nodes 582.82
159.33
582.79
159.32
-28 B
-8 B

@cabanier
Copy link
Contributor Author

cabanier commented May 28, 2025

I verified that with this change, all the surfaces are correctly sized and multisampled:
Surface 1 | 1500x1000 | color 32bit, depth 24bit, stencil 0 bit, MSAA 1, Mode: 1 (HwBinning) | 4 768x512 bins ( 4 rendered) | 0.33 ms | 10 stages : Binning : 0.147ms Render : 0.142ms StoreColor : 0.008ms Blit : 0.005ms
Surface 2 | 1500x1000 | color 32bit, depth 0 bit, stencil 0 bit, MSAA 1, Mode: 2 (SwBinning) | 2 768x1024 bins ( 2 rendered) | 0.17 ms | 4 stages : Render : 0.162ms StoreColor : 0.004ms
Surface 3 | 800 x800 | color 32bit, depth 24bit, stencil 0 bit, MSAA 1, Mode: 1 (HwBinning) | 2 480x800 bins ( 2 rendered) | 0.10 ms | 6 stages : Binning : 0.025ms Render : 0.054ms StoreColor : 0.004ms Blit : 0.004ms
Surface 4 | 800 x800 | color 32bit, depth 0 bit, stencil 0 bit, MSAA 1, Mode: 2 (SwBinning) | 1 864x800 bins ( 1 rendered) | 0.06 ms | 2 stages : Render : 0.06ms StoreColor : 0.002ms
Surface 5 | 1280x800 | color 32bit, depth 24bit, stencil 0 bit, MSAA 1, Mode: 2 (SwBinning) | 3 480x800 bins ( 4 rendered) | 0.11 ms | 7 stages : Render : 0.08ms StoreColor : 0.007ms Blit : 0.004ms
Surface 6 | 1280x800 | color 32bit, depth 0 bit, stencil 0 bit, MSAA 1, Mode: 2 (SwBinning) | 2 864x864 bins ( 2 rendered) | 0.13 ms | 4 stages : Render : 0.117ms StoreColor : 0.004ms
Surface 7 | 1680x1760 | color 32bit, depth 24bit, stencil 0 bit, MSAA 4, Mode: 1 (HwBinning) | 63 192x256 bins ( 63 rendered) | 2.68 ms | 129 stages : Binning : 0.25ms Render : 1.161ms StoreColor : 0.132ms Blit : 0.005ms Preempt : 0.743ms
Surface 8 | 1680x1760 | color 32bit, depth 24bit, stencil 0 bit, MSAA 4, Mode: 2 (SwBinning) | 66 288x160 bins ( 36 rendered) | 1.03 ms | 71 stages : Render : 0.563ms StoreColor : 0.277ms Blit : 0.006ms
Surface 9 | 1680x1760 | color 32bit, depth 24bit, stencil 0 bit, MSAA 4, Mode: 0 (Direct) | 1 1680x1760 bins ( 1 rendered) | 0.00 ms | 1 stages : Render : 0.002ms

@mrdoob mrdoob changed the title remove _forceViewport WebGPURenderer: remove _forceViewport May 28, 2025
@mrdoob mrdoob changed the title WebGPURenderer: remove _forceViewport WebGPURenderer: Remove _forceViewport May 28, 2025
@mrdoob mrdoob added this to the r177 milestone May 28, 2025
@mrdoob mrdoob requested a review from Mugen87 May 28, 2025 05:40
@Mugen87
Copy link
Collaborator

Mugen87 commented May 28, 2025

Awesome! That is what I was hoping for! The PR fixes the calls of state.viewport() and the dimensions of internal framebuffer target is now kept in sync with the output render target.

Or there calls to setSize I can remove?

With this PR, calls like below should be obsolete since the dimensions are now extracted from the output render target. But I suggest we do this with a second PR (maybe targeting r178).

renderer.setPixelRatio( 1 );
renderer.setSize( glBaseLayer.framebufferWidth, glBaseLayer.framebufferHeight, false );

@Mugen87 Mugen87 merged commit 8797943 into mrdoob:dev May 28, 2025
12 checks passed
@cabanier
Copy link
Contributor Author

With this PR, calls like below should be obsolete since the dimensions are now extracted from the output render target. But I suggest we do this with a second PR (maybe targeting r178).

ok. Will you let me know when that is?

@cabanier cabanier deleted the xr-cleanup branch May 28, 2025 17:17
@mrdoob
Copy link
Owner

mrdoob commented May 28, 2025

You can see the dates for upcoming releases here:

https://github.com/mrdoob/three.js/milestones

(I'm a day late for the one this month. I'll probably be two days late...)

RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
RuthySheffi pushed a commit to RuthySheffi/three.js that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants