Skip to content

docs(skinning): #819 close-out — recorded comparison run + forearm-twist captures - #834

Merged
fernandotonon merged 1 commit into
masterfrom
docs/skinning-quality-819-closeout
Jul 10, 2026
Merged

docs(skinning): #819 close-out — recorded comparison run + forearm-twist captures#834
fernandotonon merged 1 commit into
masterfrom
docs/skinning-quality-819-closeout

Conversation

@fernandotonon

@fernandotonon fernandotonon commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Fills the last two #819 acceptance items (docs only):

  • Recorded run in docs/SKINNING_QUALITY.md: --evaluate / --compare metrics against the artist-skinned bandit reference (119 bones / 90k verts) for SkinTokens+localisation and GeodesicVoxel, with the interesting datum that the artist reference itself scores the highest bleed (deliberate non-local weights on twist/accessory bones).
  • Forearm-twist comparison: reproducible dense-bar fixture + headless qtmesh pose/turntable renders showing LBS volume collapse at the blend zone (rest vs 90°). The DQS side is blocked on macOS legacy-GL by newly-filed DQS skinning display renders nothing on macOS (legacy GL): entity disappears when the imprinted RTSS technique regenerates #833 (imprinted technique renders nothing) — documented as a known issue; the toggle's imprint path stays covered by SkinningDisplayTest on Linux CI.

Closes out #819 together with PRs #829 / #830 / #831.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added recorded skinning-quality comparison results with metrics and interpretation.
    • Documented a reproducible forearm-twist visual test, including rest and 90° twist comparisons.
    • Added a note about a macOS-specific issue affecting DQS rendering.
  • Chores
    • Updated ignore rules to exclude generated documentation images from version control.

…BS captures (#819 close-out)

Fills the two remaining #819 Slice E acceptance items:
- Recorded run: --evaluate/--compare against the artist-skinned bandit
  reference (SkinTokens+localisation L1 1.224 / bleed 0.076; GVB 1.093
  / 0.028; the artist reference itself scores bleed 0.143 — artists
  paint deliberate non-local weights). Mixamo-exported references slot
  into the same commands.
- Forearm-twist comparison: reproducible dense-bar fixture (auto-rig
  generic template + GVB + injected glTF Twist animation), headless
  pose/turntable renders showing LBS volume loss at the blend zone.
  DQS side blocked on macOS legacy-GL by the new #833 (entity renders
  nothing with the imprinted technique) — documented as known issue.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The changes document dated skinning-quality measurements and a reproducible forearm-twist render comparison, including a macOS DQS limitation. Generated comparison images and the docs/img directory are added to .gitignore.

Changes

Skinning quality documentation

Layer / File(s) Summary
Recorded skinning metrics
docs/SKINNING_QUALITY.md
Adds dated bandit.fbx comparison metrics and interpretation of bone differences, bleed, and L1 error.
Forearm-twist comparison procedure
docs/SKINNING_QUALITY.md, .gitignore
Documents reproducible rest and 90° twist renders, embedded comparisons, the macOS DQS limitation, and ignored generated images.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the summary and technical details, but it omits the template's section headings and checklist formatting. Reformat the description into the repo template with Summary, Technical Details, and any applicable PS1 runtime rip checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the docs-only skinning comparison and forearm-twist capture changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/skinning-quality-819-closeout

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 @.gitignore:
- Around line 152-154: Replace the negated patterns for docs/img and its
generated PNG files with explicit ignore patterns, removing the ! prefixes so
the generated comparison images remain ignored.

In `@docs/SKINNING_QUALITY.md`:
- Around line 138-145: The forearm comparison documentation omits the rest-pose
render and image naming steps. Update the command block in the skinning quality
guide to render both the rest pose and 90° Twist pose, then explicitly document
how the turntable outputs are renamed or placed as img/twist_bar_rest_lbs.png
and img/twist_bar_90_lbs.png to match the referenced images.
- Around line 138-141: Specify the fenced code block language in the command
example by changing its opening fence to ```bash, while leaving the contained
commands unchanged.
🪄 Autofix (Beta)

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3adebf07-dabf-4024-8c15-ef997ed81a2a

📥 Commits

Reviewing files that changed from the base of the PR and between 3b18e97 and 08afe1b.

⛔ Files ignored due to path filters (2)
  • docs/img/twist_bar_90_lbs.png is excluded by !**/*.png
  • docs/img/twist_bar_rest_lbs.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • .gitignore
  • docs/SKINNING_QUALITY.md

Comment thread .gitignore
Comment on lines +152 to +154
!docs/img
!docs/img/twist_bar_rest_lbs.png
!docs/img/twist_bar_90_lbs.png

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Do not use negation rules for generated images.

The ! prefix unignores these paths, so docs/img and both PNGs remain eligible for tracking. If these generated comparison images should be ignored, use explicit ignore patterns without !, such as docs/img/.

🤖 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 @.gitignore around lines 152 - 154, Replace the negated patterns for docs/img
and its generated PNG files with explicit ignore patterns, removing the !
prefixes so the generated comparison images remain ignored.

Comment thread docs/SKINNING_QUALITY.md
Comment on lines +138 to +141
```
qtmesh pose twist_bar_rig.gltf2 --animation Twist --time 0.5 -o bar_90.stl
qtmesh turntable bar_90.stl -o bar_90_%02d.png --frames 4
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Specify the fenced block language.

Change the opening fence to ```bash to satisfy MD040.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 138-138: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/SKINNING_QUALITY.md` around lines 138 - 141, Specify the fenced code
block language in the command example by changing its opening fence to ```bash,
while leaving the contained commands unchanged.

Source: Linters/SAST tools

Comment thread docs/SKINNING_QUALITY.md
Comment on lines +138 to +145
```
qtmesh pose twist_bar_rig.gltf2 --animation Twist --time 0.5 -o bar_90.stl
qtmesh turntable bar_90.stl -o bar_90_%02d.png --frames 4
```

| rest (LBS) | 90° twist (LBS) |
|---|---|
| ![rest](img/twist_bar_rest_lbs.png) | ![90° LBS](img/twist_bar_90_lbs.png) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the forearm comparison commands reproduce the referenced images.

The documented commands only render the 90° pose to bar_90_%02d.png; they do not render the rest pose or produce the referenced img/twist_bar_rest_lbs.png and img/twist_bar_90_lbs.png files. Add the rest-pose command and document the exact output/rename step.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 138-138: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/SKINNING_QUALITY.md` around lines 138 - 145, The forearm comparison
documentation omits the rest-pose render and image naming steps. Update the
command block in the skinning quality guide to render both the rest pose and 90°
Twist pose, then explicitly document how the turntable outputs are renamed or
placed as img/twist_bar_rest_lbs.png and img/twist_bar_90_lbs.png to match the
referenced images.

@fernandotonon
fernandotonon merged commit fdf2a20 into master Jul 10, 2026
13 checks passed
@fernandotonon
fernandotonon deleted the docs/skinning-quality-819-closeout branch July 10, 2026 20:49
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant