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

Addons: Remove GPUStatsPanel. #29317

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Addons: Remove GPUStatsPanel. #29317

merged 2 commits into from
Sep 5, 2024

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Sep 4, 2024

Related issue: #29295

Description

This PR removes the addon GPUStatsPanel. For more detailed performance monitoring, it's best to use https://github.com/RenaudRohlinger/stats-gl since it works for both WebGL and WebGPU.

As mentioned earlier, ideally stats-gl replaces the original stats.js in the examples at some point. Since there seems to be some issues in context of Safari and M processors (see #28978), it's probably best to wait a bit until these problems have been resolved.

@Mugen87 Mugen87 changed the title Addons: Remove GPUStatsPanel. Addons: Remove GPUStatsPanel. Sep 4, 2024
@Mugen87 Mugen87 added this to the r169 milestone Sep 4, 2024
@gkjohnson
Copy link
Collaborator

Does it make sense to remove this before stats-gl properly replaces stats.js? I still use this plugin and have used it to benchmark changes to some of the examples that it's removed from in this PR.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Sep 4, 2024

Can you switch instead to stats-gl on your side? If you put the following into an example's import map, you can import via import Stats from 'stats-gl'; and use it as a replacement of stats.js.

"stats-gl": "https://cdn.jsdelivr.net/npm/[email protected]/dist/main.js"

I have the feeling this is better than retaining GPUStatsPanel which is simply rendundant.

@gkjohnson
Copy link
Collaborator

I'm just suggesting to not remove stats.js related components until stats.js is removed from the project. But I see that stats-gl is basically the same thing.

@mrdoob
Copy link
Owner

mrdoob commented Sep 5, 2024

How about replacing GPUStatsPanel with stats-gl in those examples instead?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Sep 5, 2024

Done!

@mrdoob mrdoob merged commit b6cb1eb into mrdoob:dev Sep 5, 2024
11 checks passed
@RenaudRohlinger
Copy link
Collaborator

Regarding stats-gl, I’m considering disabling the GPU metrics by default.
Since this is the only metric currently causing issues and is quite advanced, its default state might be better set to off, especially given that it’s not entirely accurate. The CPU metric, on the other hand, is both stable and highly useful, which could be sufficient to drive adoption and facilitate the transition from stats to stats-gl. /cc @Mugen87

@RenaudRohlinger
Copy link
Collaborator

Released [email protected] which doesn't have the GPU metrics by default.

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.

4 participants