ENH: Clean-up docs and add lighting to USD stages - #110
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughThe PR updates USD viewing documentation for Omniverse RTX workflows, adds distant-light creation to framing-camera generation, relocates the clinical-use disclaimer, removes related CSS, and sets six PCA modes in a lung-fitting tutorial. ChangesUSD visualization and documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the USD export/viewing experience by adding a default light to framed USD stages and revising documentation to focus on Omniverse/RTX viewing, while also making a small tutorial parameter update.
Changes:
- Add an optional
UsdLux.DistantLightalongside the framing camera in USD stages. - Revise USD viewing docs to emphasize Omniverse Kit + RTX and using the authored
/World/Camera. - Update the lung statistical-model fitting tutorial to explicitly set
pca_number_of_modes.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tutorials/tutorial_07_lung_fit_statistical_model_to_patient.py | Passes an explicit PCA mode count when enabling PCA registration. |
| src/physiotwin4d/vtk_to_usd/usd_utils.py | Adds a distant light authored with the framing camera transform. |
| README.md | Rewords the clinical-use disclaimer text. |
| experiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.py | Updates viewing guidance text toward Omniverse/RTX. |
| docs/viewing_usd.rst | Rewrites USD viewing guidance around Omniverse Kit + RTX and the scene camera. |
| docs/tutorials.rst | Updates the “Viewing USD” pointer text to match Omniverse-only guidance. |
| docs/troubleshooting.rst | Updates “USD Not Animating” steps to emphasize Omniverse + scene camera. |
| docs/quickstart.rst | Adjusts USD visualization guidance to match Omniverse/RTX recommendations. |
| docs/installation.rst | Fixes/install guidance for viewing USD (RTX + scene camera reference). |
| docs/index.rst | Updates landing page copy and removes the separate clinical-notice block. |
| docs/faq.rst | Revises Omniverse guidance to emphasize RTX material evaluation. |
| docs/developer/usd_generation.rst | Documents the new framing light behavior next to the framing camera. |
| docs/cli_scripts/heart_gated_ct.rst | Updates “Viewing Results” steps to RTX + /World/Camera. |
| docs/cli_scripts/byod_tutorials.rst | Updates USD viewing steps to RTX + /World/Camera. |
| docs/architecture.rst | Removes the duplicated clinical warning block. |
| docs/_static/custom.css | Removes CSS for the deleted clinical-notice component. |
Suppressed comments (1)
docs/index.rst:31
- Same mojibake issue here ("—"), which will render incorrectly. Use an em dash entity instead.
<p>Install the package, then clone the repository — the tutorial scripts do not ship in the wheel.</p>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 36.55% 36.60% +0.05%
==========================================
Files 72 72
Lines 8503 8510 +7
==========================================
+ Hits 3108 3115 +7
Misses 5395 5395
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/quickstart.rst (1)
7-7: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRetain the clinical-use warning on standalone workflow pages.
The landing-page warning does not appear when a user opens a deep link to a workflow page.
docs/quickstart.rst#L7-L7: restore a local.. warning::before the prerequisites.docs/cli_scripts/byod_tutorials.rst#L12-L12: restore a local.. warning::before the installation instructions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/quickstart.rst` at line 7, Restore a local .. warning:: containing the clinical-use warning before the prerequisites in docs/quickstart.rst at lines 7-7 and before the installation instructions in docs/cli_scripts/byod_tutorials.rst at lines 12-12, so each standalone workflow page displays the warning when opened directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/index.rst`:
- Line 17: Replace the mojibake `—` in both changed homepage strings in
docs/index.rst with a correctly encoded UTF-8 em dash or `—`, preserving
the surrounding text.
In `@docs/viewing_usd.rst`:
- Around line 11-13: Update docs/viewing_usd.rst lines 11-13 to describe NVIDIA
Omniverse Kit as the recommended viewer for full OmniSurface and RTX rendering,
not the only viewer. Update docs/faq.rst lines 29-32 to begin the answer with
“No. Omniverse is optional.”
- Around line 24-31: Update the kit-app-template command instructions in the USD
Composer setup text to use ./repo.sh on Linux/macOS and .\repo.bat on Windows
for template new, build, and launch, replacing the bare repo command while
preserving the existing command sequence.
In `@src/physiotwin4d/vtk_to_usd/usd_utils.py`:
- Around line 448-450: Update the add_framing_camera documentation in
src/physiotwin4d/vtk_to_usd/usd_utils.py at lines 448-450 and the
generated-light documentation in docs/developer/usd_generation.rst at lines
150-154 to use identical wording: the DistantLight is created only when valid,
non-degenerate bounds exist; otherwise, no light is authored.
- Around line 542-547: Before defining the light in the camera/light creation
flow, validate that light_path does not equal camera_path and that any existing
prim at light_path is compatible with UsdLux.DistantLight; return an error for
either collision. Keep UsdLux.DistantLight.Define and the subsequent light setup
unchanged for valid, non-colliding paths.
In `@tutorials/tutorial_07_lung_fit_statistical_model_to_patient.py`:
- Line 125: Update the tutorial test for this workflow to assert that the PCA
registration uses exactly six modes, checking workflow.pca_number_of_modes or
pca_registrar.pca_number_of_modes after execution. Keep the existing execution
and screenshot assertions unchanged so CI fails if the setting is reverted.
---
Outside diff comments:
In `@docs/quickstart.rst`:
- Line 7: Restore a local .. warning:: containing the clinical-use warning
before the prerequisites in docs/quickstart.rst at lines 7-7 and before the
installation instructions in docs/cli_scripts/byod_tutorials.rst at lines 12-12,
so each standalone workflow page displays the warning when opened directly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 224ae773-f23d-43ed-9176-a749a5064ffa
📒 Files selected for processing (16)
README.mddocs/_static/custom.cssdocs/architecture.rstdocs/cli_scripts/byod_tutorials.rstdocs/cli_scripts/heart_gated_ct.rstdocs/developer/usd_generation.rstdocs/faq.rstdocs/index.rstdocs/installation.rstdocs/quickstart.rstdocs/troubleshooting.rstdocs/tutorials.rstdocs/viewing_usd.rstexperiments/Convert_VTK_To_USD/convert_vtk_to_usd_using_class.pysrc/physiotwin4d/vtk_to_usd/usd_utils.pytutorials/tutorial_07_lung_fit_statistical_model_to_patient.py
💤 Files with no reviewable changes (2)
- docs/architecture.rst
- docs/_static/custom.css
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/physiotwin4d/vtk_to_usd/usd_utils.py:436
- Repo style uses
Optional[X]instead ofX | Nonein type hints (including return annotations). Consider updating the entireadd_framing_camerasignature accordingly so it matches the rest of the codebase's typing style.
bounds_min: tuple[float, float, float] | None = None,
bounds_max: tuple[float, float, float] | None = None,
src/physiotwin4d/vtk_to_usd/usd_utils.py:13
- Project conventions prefer
Optional[X]overX | None. Sinceadd_framing_camerawill switch toOptional[...]type hints, the typing import should includeOptionalto keep mypy happy.
This issue also appears on line 435 of the same file.
from pxr import Gf, Sdf, Usd, UsdGeom, UsdLux, Vt
src/physiotwin4d/vtk_to_usd/usd_utils.py:543
- Same path-construction issue as for the camera: prefer
Sdf.Path(...).AppendChild(...)rather thanrstrip('/')inside an f-string expression.
light_path = f"{parent_path.rstrip('/')}/{light_name}"
src/physiotwin4d/vtk_to_usd/usd_utils.py:548
- The converter now always authors a
/World/DistantLightwhen bounds are valid. There are existing USD conversion tests, but none assert this new prim exists (or that it is skipped for empty stages). Please add a small regression test to validate the presence/absence and default intensity/angle so future refactors don't silently drop lighting.
light = UsdLux.DistantLight.Define(stage, light_path)
light_xformable = UsdGeom.Xformable(light.GetPrim())
light_xformable.ClearXformOpOrder()
light_xformable.AddTransformOp().Set(camera_to_world)
light.CreateIntensityAttr().Set(float(light_intensity))
Summary by CodeRabbit
Documentation
Enhancements