refactor(core): remove legacy WITH_(SLOG|STTF) and image-output paths#321
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".
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.
plexoos
force-pushed
the
remove-with-slog
branch
7 times, most recently
from
bd55f66 to
8c302eb
Compare
|
@codex review |
plexoos
changed the title
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 👍 / 👎.
There was a problem hiding this comment.
Confirmed, the observation is technically correct: this PR changes renderer snapshots from .jpg image output to .npy frame dumps while some legacy CSGOptiX scripts still assume .jpg frames. That said, preserving .jpg output is not the direction of this PR; in-repo visualization/image publishing is being retired. I’ll treat the remaining JPG-oriented scripts/docs as follow-up cleanup rather than restoring the old SIMG/STTF image path.
|
@copilot review |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
plexoos
force-pushed
the
remove-with-slog
branch
2 times, most recently
from
6670bed to
0b4c8f1
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
plexoos
changed the title
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_*headersu4optical-process instrumentation to the supported in-tree path.npyframe dumps with metadataWhy
This codebase no longer aims to support the old workflow with 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.