-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Transparent Pixel Support and Minor Fixes. #22
base: master
Are you sure you want to change the base?
Conversation
- Add builder function.
Single char escape sequences
Allow displaying transparent pixels.
Reviewer's Guide by SourceryThis pull request introduces transparent pixel support and a new option for escaping ANSI color codes on each character. The changes include updates to the rendering logic, command-line options, charsets, and dependency versions, along with code formatting improvements. Updated Class Diagram for ImageRenderer and RenderOptionsclassDiagram
class ImageRenderer {
- resource: &DynamicImage
- options: RenderOptions
+ get_char_for_pixel(pixel: Rgba<u8>, maximum: f64): &str
+ get_opacity_percent(pixel: Rgba<u8>): f64
+ get_grayscale(pixel: Rgba<u8>): f64
}
class RenderOptions {
- width: Option<u32>
- height: Option<u32>
- colored: bool
- escape_each_colored_char: bool
- invert: bool
- charset: &[&str]
+ escape_each_colored_char(escape: bool): Self
+ invert(invert: bool): Self
+ ...
}
ImageRenderer --> RenderOptions : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Stattek - I've reviewed your changes - here's some feedback:
Overall Comments:
- Extract the 0.95 opacity threshold into a named constant to make its purpose clearer.
- Abstract the repeated ANSI color prefix logic into a helper function to reduce duplication between writer and buffer handling.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@@ -24,6 +29,10 @@ impl ImageRenderer<'_> { | |||
}] | |||
} | |||
|
|||
fn get_transparency_percent(&self, pixel: &Rgba<u8>) -> f64 { | |||
pixel[3] as f64 / u8::MAX as f64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using u8::MAX is smarter than plain 255.
Just a note to myself
Thank you for your PR. I'm waiting for the question |
Adds ability to render PNGs with transparent pixels (pixels that are at least 95% transparent will be rendered with 0 index in charset).
image
to version0.25.5
cargo fmt
on src files.jp2a
program.Summary by Sourcery
New Features: