fix #34026: prevent unlimited memory for image via maxScaledImageDim#34038
Conversation
📝 WalkthroughWalkthroughChangesA new configuration option Related PRs: None identified. Suggested labels: engraving, rendering Suggested reviewers: None identified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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.
🧹 Nitpick comments (1)
src/engraving/rendering/score/tdraw.cpp (1)
2067-2074: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the
maxDim <= 0disable semantics.
useDirectDrawsilently skips the size-based clamp whenevermaxDim <= 0. This is a reasonable escape hatch but isn't documented anywhere (interface, impl, or here), so a future reader/config author could set 0 expecting "always direct-draw" or misunderstand the behavior.🤖 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 `@src/engraving/rendering/score/tdraw.cpp` around lines 2067 - 2074, Document the `maxScaledImageDim()` disable behavior in `tdraw.cpp` around `useDirectDraw` so it’s clear that `maxDim <= 0` turns off the size-based direct-draw clamp. Add a brief note in the relevant implementation/comments near the `useDirectDraw` decision, and if there is a matching configuration/interface declaration for `maxScaledImageDim()`, document the same semantics there so readers understand that 0 or negative means “no limit” rather than “always direct-draw.”
🤖 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.
Nitpick comments:
In `@src/engraving/rendering/score/tdraw.cpp`:
- Around line 2067-2074: Document the `maxScaledImageDim()` disable behavior in
`tdraw.cpp` around `useDirectDraw` so it’s clear that `maxDim <= 0` turns off
the size-based direct-draw clamp. Add a brief note in the relevant
implementation/comments near the `useDirectDraw` decision, and if there is a
matching configuration/interface declaration for `maxScaledImageDim()`, document
the same semantics there so readers understand that 0 or negative means “no
limit” rather than “always direct-draw.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63ada2b6-a937-4c8a-b222-1ce221505e86
📒 Files selected for processing (5)
src/engraving/iengravingconfiguration.hsrc/engraving/internal/engravingconfiguration.cppsrc/engraving/internal/engravingconfiguration.hsrc/engraving/rendering/score/tdraw.cppsrc/engraving/tests/mocks/engravingconfigurationmock.h
|
🟢 TESTED on macOS 26.3, Win 11, Ubuntu 20 |
Resolves: #34026