Skip to content
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 strokewidth parameter to image_annotate #388

Merged
merged 3 commits into from
Nov 5, 2023
Merged

add strokewidth parameter to image_annotate #388

merged 3 commits into from
Nov 5, 2023

Conversation

retowyss
Copy link
Contributor

@retowyss retowyss commented Nov 3, 2023

Adding missing strokewidth parameter to image_annotate.

library(magick)

img <- image_blank(480, 320)

image_annotate(img, "Strokewidth Test", size = 60, strokecolor = "red", strokewidth = 1)
image_annotate(img, "Strokewidth Test", size = 60, strokecolor = "red", strokewidth = 2)

@@ -351,6 +352,8 @@ XPtrImage magick_image_annotate( XPtrImage input, Rcpp::CharacterVector text, co
draw.push_back(Magick::DrawableTextAntialias(true));
if(strokecolor.size())
draw.push_back(Magick::DrawableStrokeColor(Color(strokecolor[0])));
if(strokewidth != 0)
Copy link
Member

Choose a reason for hiding this comment

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

Does NULL (the default) really become 0 when the R integer vector is coerced to a double?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. I believe the default in magick is 1 and I've set the default in image_annotate to 1. I haven't tested behavior if 0 is allowed. I haven't tested behavior if strokecolor isn't set but strokewidth is set either. So maybe some additional checks may be necessary; I don't have much familiarity with magick. I assume non-integer values may be non-sense but then I wasn't sure whether it's the job of this package to catch that.

@jeroen jeroen merged commit dc5f0e7 into ropensci:master Nov 5, 2023
8 checks passed
@jeroen
Copy link
Member

jeroen commented Nov 5, 2023

Thanks!

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.

2 participants