Load SVG images, rasterized at a scale - #60
Merged
Merged
Conversation
The squash of #59 lost the for loop's closing brace, so main did not compile. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Image recognizes SVG by declared type or by sniffing and rasterizes it through SDL_image's nanosvg. Image::imageWithSVG and initWithSVG take a scale, pixels per point such as the window's pixel density, so a vector asset drawn into a frame of its intrinsic size stays sharp on a high density display; Image::scale records it and Image::size reports points, so views sized to an image are unaffected. Raster images keep a scale of 1. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A generated libtool wrapper script with a hard-coded developer-local path is committed, and the new public initWithSVG input validation relies on assert rather than runtime error handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR restores a missing brace in Text::applyTransform to fix compilation, and adds SVG image loading with rasterization at a caller-provided scale so vector assets remain sharp on high-DPI displays while Image::size continues to report point sizes for layout.
Changes:
- Fix
Text.ccompilation by restoring a dropped closing brace inapplyTransform. - Add SVG recognition (declared type or sniffing) and scaled rasterization via new
Image::imageWithSVG/initWithSVG, plusImage::scaleand point-basedImage::size. - Add an
Imagetest case to the autotools test suite covering intrinsic SVG load, scaled rasterization, and failure on invalid SVG input.
File summaries
| File | Description |
|---|---|
| Tests/ObjectivelyMVC/Makefile.am | Adds the new Image test to the autotools TESTS list. |
| Tests/ObjectivelyMVC/Image.c | New Check-based tests for SVG loading/rasterization behavior and scale semantics. |
| Tests/ObjectivelyMVC/Image | Newly added libtool wrapper script (appears to be a generated build artifact). |
| Sources/ObjectivelyMVC/Text.c | Restores missing closing brace in applyTransform loop. |
| Sources/ObjectivelyMVC/Image.h | Documents SVG scaling behavior; adds scale field and SVG-loading API declarations. |
| Sources/ObjectivelyMVC/Image.c | Implements SVG sniffing, rasterization helpers, scale tracking, and point-sized size(). |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits:
forloop's closing brace inapplyTransform, somaindoes not compile. Restored.Imagerecognizes SVG by declared type or by sniffing (IMG_isSVG) and rasterizes it through SDL_image's bundled nanosvg.Image::imageWithSVG/initWithSVGtake ascale(pixels per point, e.g. the window's pixel density) so a vector asset drawn into a frame of its intrinsic size stays sharp on a high density display.Image::scalerecords it andImage::sizereports points, soImageView::setImagesizing is unaffected. Raster images keep a scale of 1. PlainimageWithBytes/resources load SVG at scale 1.Tests:
Tests/ObjectivelyMVC/Image.ccovers intrinsic load, 2x rasterization with point-sizedsize, raster scale, and a garbage document failing. Added to the autotools suite only, matching the Text tests.Motivation: Quetoo is re-authoring its HUD item icons as SVG (jdolan/quetoo#1026).
🤖 Generated with Claude Code