refactor(core): remove legacy standalone and image-output paths#321
refactor(core): remove legacy standalone and image-output paths#321plexoos wants to merge 4 commits into
Conversation
plexoos
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aa8869bc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #include <sstream> | ||
|
|
||
| #ifdef WITH_SLOG | ||
| #include "plog/Severity.h" |
There was a problem hiding this comment.
This unconditional plog/Severity.h include breaks the documented STANDALONE/header-only usage of OpticksPhoton.hh: the standalone compile path in sysrap/tests/OpticksPhotonSTANDALONETest.sh uses gcc $name.cc -I.. and now fails with fatal error: plog/Severity.h: No such file or directory unless callers manually add plog include paths. Before this change, the WITH_SLOG guard avoided that external dependency for standalone consumers, so this is a functional regression for non-CMake or minimal builds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR simplifies the build by removing the deprecated WITH_SLOG and WITH_STTF guards that were effectively always enabled, and by making the associated logging/font code paths unconditional across SysRap/CSG-related code.
Changes:
- Removed obsolete
WITH_SLOG/WITH_STTFcompile definitions from CMake and PTX build settings. - Made
SLOGusage inCSG::CUandOpticksPhotonunconditional, and always registeredSTTFTest.cc. - Deleted legacy SLOG documentation files and replaced them with short inline header notes.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
u4/CMakeLists.txt |
Minor cleanup of stale blank line near compile definitions. |
sysrap/tests/CMakeLists.txt |
Registers STTFTest.cc unconditionally in SysRap tests. |
sysrap/SLOG.rst |
Deletes legacy standalone SLOG documentation. |
sysrap/SLOG.hh |
Replaces external doc reference with short inline logging note. |
sysrap/SLOG_review.rst |
Deletes old SLOG review/design notes. |
sysrap/OpticksPhoton.hh |
Makes plog severity declaration/header dependency unconditional. |
sysrap/OpticksPhoton.cc |
Makes SLOG include and LEVEL definition unconditional. |
sysrap/OPTICKS_LOG.hh |
Adds inline note describing SLOG.hh / SLOG_INIT.hh roles. |
sysrap/CMakeLists.txt |
Removes deprecated guard variables/definitions; always installs STTF headers. |
qudarap/CMakeLists.txt |
Removes stale local-debugging comment block. |
CSGOptiX/CMakeLists.txt |
Drops deprecated guard definitions from PTX object compilation. |
CSG/CU.h |
Makes plog severity dependency unconditional in CU. |
CSG/CU.cc |
Makes SLOG include, level, and logging calls unconditional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -27,9 +25,7 @@ struct OpticksPhoton | |||
| struct SYSRAP_API OpticksPhoton | |||
| #endif | |||
| { | |||
| #ifdef WITH_SLOG | |||
| static const plog::Severity LEVEL ; | |||
plexoos
force-pushed
the
remove-with-slog
branch
6 times, most recently
from
d153937 to
bd55f66
Compare
Remove the remaining WITH_SLOG compile definitions from sysrap and CSGOptiX. Delete the conditional logging includes, LEVEL declarations, and debug-only LOG calls in CSG::CU and OpticksPhoton now that the flag no longer controls functional behavior. This simplifies the build configuration and removes dead code without changing runtime logic.
- drop the STTF/stb_truetype implementation and font path config - remove SIMG/Frame/CSGOptiX annotation hooks and simplify SRenderer::snap - delete STTF-specific tests and stop SIMGTest from expecting text overlays
- drop the SIMG/SIMG_Frame wrappers and bundled stb image headers - switch SGLFW and CSGOptiX snapshot output from jpg/png to raw npy buffers - remove SIMG-dependent tests and the qudarap image rotation test
|
@codex review |
plexoos
changed the title
Cpp-Linter Report
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7678d43dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| bool unique = true ; | ||
| const char* outpath = SEventConfig::OutPath(stem, -1, ".jpg", unique ); | ||
| const char* outpath = SEventConfig::OutPath(stem, -1, ".npy", unique); |
There was a problem hiding this comment.
Changing render_save_ to emit .npy (SEventConfig::OutPath(..., ".npy", ...)) turns renderer snapshots into raw arrays instead of image files, which breaks existing CSGOptiX workflows that consume .jpg frames (for example CSGOptiX/cxr_flight.sh expects *_00000.jpg and invokes ffmpeg_jpg_to_mp4.sh, and CSGOptiX/grab.sh syncs *.jpg). In practice, users pressing snap in interactive/render scripts will no longer produce the files those downstream tools are built around.
Useful? React with 👍 / 👎.
This branch finishes removing several legacy paths that are no longer part of the supported simphony workflow.
What changed
WITH_SLOGandWITH_STTFguard logicSTTFtext-annotation code and related testsSIMGimage-output wrappers and bundledstb_*headersSTANDALONE/header-onlyOpticksPhotonpath and its standalone testu4optical-process instrumentation to the supported in-tree path.npyframe dumps with metadataWhy
This codebase no longer aims to support the old standalone/header-only workflow or in-repo visualization/image publishing paths. Those branches had become stale, added dead conditional code, and were already drifting out of sync with the supported build.
Most of the large deletion count in this PR comes from removing bundled image/font helper headers and their tests.