Repository navigation
CLI: log warnings to stderr by default - #1651
Open
MatejKozlovsky wants to merge 1 commit into
Open
MatejKozlovsky wants to merge 1 commit into
MatejKozlovsky wants to merge 1 commit into
Conversation
The CLI installed no logger, so GPU warnings (a declined frame, out of memory, fallback to the CPU) were dropped and an export silently rendered on the CPU. Install the engine logger with a `warn` default (RUST_LOG still overrides). Split out of storytold#1415.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CLI never installed a logger, so
log::warn!from the GPU path (a frame the GPU declines, device out of memory, falling back to the CPU) went nowhere and an export could quietly render on the CPU with no hint why.This installs the engine logger in the CLI with a
warndefault;RUST_LOGstill overrides it. The app keeps its current default (warn,effectcraft=info) throughlogging::install(), which now just calls the newinstall_with(DEFAULT_FILTER).Split out of #1415 as suggested; no other changes from that PR.
apps/effectcraft-cli/src/main.rs:logging::install_with("warn")at the start ofmain(on top of CLI: an argument that is not Unicode is a usage error, not a panic #1357'sargs_oschanges)crates/engine/src/logging.rs:install_with(default), plus a test for thewarnfilterTested: fmt, clippy
-D warnings(cli + engine),cargo test -p effectcraft-cli, engine logging tests, on Linux, llvmpipe.Note: a failed render now shows the error twice on stderr, the engine's
WARNline from the render queue (with the item id) plus the CLI's own error.Possible follow-up: also print the chosen adapter/backend at warn (or behind
-v) so CLI users see which GPU, if any, they got.