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

Added --gpu-index to forwardable_cli_arguments #75198

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

Bestest-Coder
Copy link
Contributor

when the gpu index is specified through the CLI, that setting will be inherited by both the editor (if started through project manager) and instances of the game started through the editor

I feel this should be default behavior, as if a user specifies a GPU to be used it should be shared from project manager to editor, and editor to debug instance. This can still be overwritten at runtime with the project setting editor/run/main_run_args

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (using --rendering-method forward_plus --gpu-index 1 on Linux), it works.

You can test this on a system with a single GPU on Linux by having llvmpipe/Lavapipe installed, then open a project that uses Forward+ or Mobile from the project manager. This works too if the project manager is started with OpenGL, which is the default unless you specify --rendering-method forward_plus.

@akien-mga akien-mga changed the title Added --gpu-index to forwardable_cli_arguments Added --gpu-index to forwardable_cli_arguments Aug 2, 2023
main/main.cpp Outdated Show resolved Hide resolved
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Seems ok, though I find all this increasingly hacky. Only some options get forwarded, and they get forwarded to different contexts. It's pretty unpredictable without reading the source code.

There may be use cases such as running the editor on an iGPU to save battery during development, while running the game on dGPU because it requires all the performance. This change would prevent doing this as the setting will propagate to both editor and game.

But I guess it's not a big deal for such advanced options.

@Bestest-Coder
Copy link
Contributor Author

You would still be able to do so, as specifying --gpu-index again in the project setting editor/run/main_run_args will overwrite it and cause the debug instance to be run on the newly specified GPU

@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

when the gpu index is specified through the CLI, that setting will
be inherited by both the editor (if started through project manager)
and instances of the game started through the editor

Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>
@Bestest-Coder
Copy link
Contributor Author

Anything else need to be done?

@akien-mga akien-mga merged commit 7ada24c into godotengine:master Aug 3, 2023
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga akien-mga changed the title Added --gpu-index to forwardable_cli_arguments Added --gpu-index to forwardable_cli_arguments Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants