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 a feature to ImageDraw.text() to stroke text #2224

Closed
wants to merge 2 commits into from
Closed

Add a feature to ImageDraw.text() to stroke text #2224

wants to merge 2 commits into from

Conversation

Irishsmurf
Copy link

@Irishsmurf Irishsmurf commented Nov 15, 2016

Fixes #2209 and #1907

Changes proposed in this pull request:

  • Adds an outline argument to ImageDraw.text() & ImageDraw.multiline_text()
  • Outline variable (Default, None) will take a colour, and stroke a 1px border around the text

This change adds an additional method argument to the draw() method
which draws the text in black, before drawing it in the color selected
in the fill parameter.

ImageDraw.text((5, 5), caption, BLACK, outline='White', font=FONT)

Alternatively - I can do this is to break out ImageDraw.text and ImageDraw.stroke_text().
Feedback welcome.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

self.draw.draw_bitmap(xy, mask, ink)

def multiline_text(self, xy, text, fill=None, font=None, anchor=None,
spacing=4, align="left"):
outline=None, spacing=4, align="left"):
Copy link
Member

Choose a reason for hiding this comment

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

Put outline at the end, because changing the order of parameters can break existing user code.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@@ -164,7 +175,7 @@ def test_render_multiline_text(self):
# to multiline_text()
draw.text((0, 0), TEST_TEXT, fill=None, font=ttf, anchor=None,
spacing=4, align="left")
draw.text((0, 0), TEST_TEXT, None, ttf, None, 4, "left")
draw.text((0, 0), TEST_TEXT, None, ttf, None, None, 4, "left")
Copy link
Member

Choose a reason for hiding this comment

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

This is an example of existing user code that is broken by changing the order of parameters. This change shouldn't be necessary.

Copy link
Author

Choose a reason for hiding this comment

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

Got it.

I've gone ahead and refactored the ImageDraw.text() method signature to no longer take in **kwargs which I believe will allow for greater flexibility in future.

Other than passing the Align and Spacing args to multiline_text, I don't believe it's using anything else (Atleast it's not documented)
Moved to explicitly declaring the keyword arguments

This change adds an additional method argument to the draw() method
which draws the text in black, before drawing it in the color selected
in the fill parameter.

Additionally due to the positional requirements enforced on the method
by using **kwargs, move to using an explicit signature with default values.

* Helps improve readability
* Allows for easier modification later.
@Irishsmurf Irishsmurf reopened this Nov 15, 2016
ink, fill = self._getink(fill)
print("Outline: %s" % outline)
Copy link
Member

Choose a reason for hiding this comment

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

Remove print

Copy link
Author

Choose a reason for hiding this comment

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

Whoops.
Addressed.

hugovk
hugovk previously approved these changes Nov 15, 2016
@Irishsmurf Irishsmurf changed the title Add a basic feature to ImageDraw.text() that enables outlining. Add a feature to ImageDraw.text() to stroke text. Nov 15, 2016
@radarhere
Copy link
Member

I'm not necessarily sold either way on this question, but have you considered updating textsize to account for the different dimensions that would be generated?

@wiredfool
Copy link
Member

I think that this is the wrong approach. At the very least, it's not adjusting the spacing due to the wider letters and the stroke is fixed at one pixel.

The right approach is going to take hooking into the freetype layer using FT_Stroker, as seen here: http://stackoverflow.com/a/28078293 .

@radarhere radarhere changed the title Add a feature to ImageDraw.text() to stroke text. Add a feature to ImageDraw.text() to stroke text Sep 8, 2017
@hugovk hugovk dismissed their stale review September 8, 2018 15:01

See comments.

@radarhere radarhere closed this Mar 31, 2019
@Irishsmurf Irishsmurf deleted the dev branch April 2, 2019 13:44
@radarhere radarhere mentioned this pull request Jul 19, 2019
@radarhere
Copy link
Member

I've created PR #3978 instead.

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

Successfully merging this pull request may close these issues.

ImageFont/ImageDraw should support text stroking
4 participants