Propagate mpl.text.Text instances to the backends and fix documentation #1081

Merged
merged 5 commits into from Nov 30, 2012

Conversation

Projects
None yet
5 participants
@pwuertz
Contributor

pwuertz commented Aug 13, 2012

This patch addresses the issue #353. Some backends could really benefit from the text alignment information that is present in mpl.text.Text instances. For instance, editing a pdf or svg would be much easier if the text elements were anchored as intended.

In contrast to the documentation of RendererBase.draw_text, a backend only receives a simple string instead of a Text instance. The patch simply adds the optional "mtext" kwarg that is used to propagate Text instances to the backends without changing the way text rendering is handled in any way. I also ran the tests from tests.py and everything seems to be fine.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 14, 2012

Owner

This is a good idea -- and yes, that's a totally bogus docstring. I think before merging this, however, I'd like to see it developed to the point of actually using the text object within the backend to see how that plays out and all the details make sense. (It could just be on one backend for now). One thing that will have to change is that the (x, y) sent to the backend needs to become the anchor point, not always the lower left as it is now.

Owner

mdboom commented Aug 14, 2012

This is a good idea -- and yes, that's a totally bogus docstring. I think before merging this, however, I'd like to see it developed to the point of actually using the text object within the backend to see how that plays out and all the details make sense. (It could just be on one backend for now). One thing that will have to change is that the (x, y) sent to the backend needs to become the anchor point, not always the lower left as it is now.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 14, 2012

Contributor

My idea was that the current way of text handling is not to be interfered with. That means the arguments (x, y) still point to the position of the lower left as calculated by matplotlib. If you want to (optionally) anchor it yourself just lookup (x, y, va, ha) from mtext where (x, y) points to the (va, ha) anchor.

I was wondering why the mtexts were not given to the backends as documented in the first place, but you are doing some latex related filtering of the string as it seems. Also, if someone implements an artist that wants to layout and draw a lot of text elements he could do so by calling draw_text without any of the texts existing as mtext instances. So it's probably a good thing to only use this as an option :)

For demonstration I could re-enable anchoring support in the pgf backend. I did a workaround once by walking through the figure text instances which was rather painful due to text elements popping up that shouldn't have been rendered. The anchoring was working with the exception of rotated texts. This would take a little bit more time to implement because matplotlib interprets anchors and rotations differently than tikz/pgf. Nevertheless, it was a huge improvement since all texts were perfectly aligned even when changing the fonts afterwards.

Contributor

pwuertz commented Aug 14, 2012

My idea was that the current way of text handling is not to be interfered with. That means the arguments (x, y) still point to the position of the lower left as calculated by matplotlib. If you want to (optionally) anchor it yourself just lookup (x, y, va, ha) from mtext where (x, y) points to the (va, ha) anchor.

I was wondering why the mtexts were not given to the backends as documented in the first place, but you are doing some latex related filtering of the string as it seems. Also, if someone implements an artist that wants to layout and draw a lot of text elements he could do so by calling draw_text without any of the texts existing as mtext instances. So it's probably a good thing to only use this as an option :)

For demonstration I could re-enable anchoring support in the pgf backend. I did a workaround once by walking through the figure text instances which was rather painful due to text elements popping up that shouldn't have been rendered. The anchoring was working with the exception of rotated texts. This would take a little bit more time to implement because matplotlib interprets anchors and rotations differently than tikz/pgf. Nevertheless, it was a huge improvement since all texts were perfectly aligned even when changing the fonts afterwards.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 14, 2012

Owner

I agree that this seems like the right approach to get text alignment information to the backend, that avoids some of the issues you saw with other approaches.

Yes -- the PGF backend would be a great test case of this. The anchoring of angled text is like nothing I've seen anywhere outside of matplotlib -- it's also a mismatch for PDF and SVG etc., but that's a tough thing to change without breaking backward compatibility. Whatever calculation on the anchor point that needs to be performed should perhaps be added to the Text object itself so that all backends can share that code.

I'd like to add support for this in SVG, PDF and PS as well (in that order, I think, because SVG being the most easily editable, it's really important there.) Hopefully between the two of us we can at least get SVG done before the freeze. I think that has the potential to be a very compelling feature for a lot of people.

Owner

mdboom commented Aug 14, 2012

I agree that this seems like the right approach to get text alignment information to the backend, that avoids some of the issues you saw with other approaches.

Yes -- the PGF backend would be a great test case of this. The anchoring of angled text is like nothing I've seen anywhere outside of matplotlib -- it's also a mismatch for PDF and SVG etc., but that's a tough thing to change without breaking backward compatibility. Whatever calculation on the anchor point that needs to be performed should perhaps be added to the Text object itself so that all backends can share that code.

I'd like to add support for this in SVG, PDF and PS as well (in that order, I think, because SVG being the most easily editable, it's really important there.) Hopefully between the two of us we can at least get SVG done before the freeze. I think that has the potential to be a very compelling feature for a lot of people.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 14, 2012

Owner

This closes #353.

Owner

mdboom commented Aug 14, 2012

This closes #353.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 14, 2012

Contributor

Rotated text is a bit tough indeed. My problem at first was to figure out the behavior in matplotlib. From what I've seen it is the axis aligned bounding box of a rotated text that is being aligned. For tikz/pgf the text anchor point is also the pivot point for rotations, but bounding box alignment must be done manually I'm afraid :/. Both definitions have their drawbacks. I don't know how SVG and PDF define the pivot points for rotations with respect to text anchors.

Maybe it's a good idea to take a step back and collect information about which pivot point definitions are easiest to support in SVG, PDF, PS and PGF. For pgf/tikz aligning text to pivot point - easy, aligning rotated bounding box - ugly but possible.

In any case I think merging this PR will make it more easy to experiment with text-align support + there are no drawbacks. The worst thing that could happen is that at some point when implementing anchoring it could appear favorable to redefine the definition of pivot points ^^.

Contributor

pwuertz commented Aug 14, 2012

Rotated text is a bit tough indeed. My problem at first was to figure out the behavior in matplotlib. From what I've seen it is the axis aligned bounding box of a rotated text that is being aligned. For tikz/pgf the text anchor point is also the pivot point for rotations, but bounding box alignment must be done manually I'm afraid :/. Both definitions have their drawbacks. I don't know how SVG and PDF define the pivot points for rotations with respect to text anchors.

Maybe it's a good idea to take a step back and collect information about which pivot point definitions are easiest to support in SVG, PDF, PS and PGF. For pgf/tikz aligning text to pivot point - easy, aligning rotated bounding box - ugly but possible.

In any case I think merging this PR will make it more easy to experiment with text-align support + there are no drawbacks. The worst thing that could happen is that at some point when implementing anchoring it could appear favorable to redefine the definition of pivot points ^^.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 14, 2012

Contributor

This will produce a SVG where no-math-not-rotated text elements are horizontally anchored. Unfortunately I found a major show-stopper concerning SVG. Nobody seems to implement vertical text alignment :(. Chrome has some support, but no luck with inkscape cairo firefox etc.

(outdated example)

Contributor

pwuertz commented Aug 14, 2012

This will produce a SVG where no-math-not-rotated text elements are horizontally anchored. Unfortunately I found a major show-stopper concerning SVG. Nobody seems to implement vertical text alignment :(. Chrome has some support, but no luck with inkscape cairo firefox etc.

(outdated example)

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 14, 2012

Owner

That's a bummer about the SVG vertical text alignment support. But I think it's still most useful to have the horizontal text alignment working. The most common use case is probably editing the title and having the text stay centered -- that works in this scenario, so it's still a win.

Owner

mdboom commented Aug 14, 2012

That's a bummer about the SVG vertical text alignment support. But I think it's still most useful to have the horizontal text alignment working. The most common use case is probably editing the title and having the text stay centered -- that works in this scenario, so it's still a win.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 17, 2012

Owner

I think this is probably ready to merge -- but we should create a new issue for the fact that PDF and PS don't use the alignment information directly.

Owner

mdboom commented Aug 17, 2012

I think this is probably ready to merge -- but we should create a new issue for the fact that PDF and PS don't use the alignment information directly.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 20, 2012

Contributor

Could you merge the PGF backend first? The mtext kwarg must be added to all backends, so once backend_pgf is merged to master I'll modify this PR and it will be ready to merge too.

Contributor

pwuertz commented Aug 20, 2012

Could you merge the PGF backend first? The mtext kwarg must be added to all backends, so once backend_pgf is merged to master I'll modify this PR and it will be ready to merge too.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Aug 20, 2012

Owner

Sure. The PGF stuff should probably get merged today.

Owner

mdboom commented Aug 20, 2012

Sure. The PGF stuff should probably get merged today.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Aug 20, 2012

Contributor

All right, I fixed up the first commit so it includes backend_pgf as well and re-ran all tests. The PR should be good to go.

Contributor

pwuertz commented Aug 20, 2012

All right, I fixed up the first commit so it includes backend_pgf as well and re-ran all tests. The PR should be good to go.

@leejjoon

This comment has been minimized.

Show comment Hide comment
@leejjoon

leejjoon Oct 7, 2012

Contributor

Just a note. The "rotation_mode"attribute of Text is meant to make the given position as an anchoring point of rotation.

Contributor

leejjoon commented Oct 7, 2012

Just a note. The "rotation_mode"attribute of Text is meant to make the given position as an anchoring point of rotation.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Oct 7, 2012

Contributor

That's perfect, thanks leejjjoon! One could check that property and support anchored rotation then.

Contributor

pwuertz commented Oct 7, 2012

That's perfect, thanks leejjjoon! One could check that property and support anchored rotation then.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Oct 7, 2012

Contributor

The alignment of text elements is now included in the pgf output. When switching fonts within TeX documents, the texts are staying aligned nicely regardless of the new font metrics.

Contributor

pwuertz commented Oct 7, 2012

The alignment of text elements is now included in the pgf output. When switching fonts within TeX documents, the texts are staying aligned nicely regardless of the new font metrics.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Oct 15, 2012

Contributor

The SVG backend now also aligns anchor-rotated text elements.

Due to the lack of 'alignment-baseline' support in most applications only the left/right/center alignment is done in SVG, the position in the vertical direction of the text is fixed. By the way, backend_svg did not handle the font decent for anchor-rotated text correctly. This is fixed now.

Example:

import matplotlib as mpl
import matplotlib.pyplot as plt
mpl.rc('svg', fonttype='none')

plt.plot(range(5), ".")
plt.xlabel("x-label")
plt.ylabel("y-label")
plt.text(2, 2, "- xXgXx -", rotation=45, rotation_mode="anchor",
         va="bottom", ha="center")
plt.savefig("edit_this_with_inkscape.svg")

This is the output of the code. You should be able to edit the y-label and a 45° rotated text in inkscape with center alignment now:
https://gist.github.com/raw/3351009/381d916bb1184135a9dabc2c963d4aff91ee9291/edit_this_with_inkscape.svg

Contributor

pwuertz commented Oct 15, 2012

The SVG backend now also aligns anchor-rotated text elements.

Due to the lack of 'alignment-baseline' support in most applications only the left/right/center alignment is done in SVG, the position in the vertical direction of the text is fixed. By the way, backend_svg did not handle the font decent for anchor-rotated text correctly. This is fixed now.

Example:

import matplotlib as mpl
import matplotlib.pyplot as plt
mpl.rc('svg', fonttype='none')

plt.plot(range(5), ".")
plt.xlabel("x-label")
plt.ylabel("y-label")
plt.text(2, 2, "- xXgXx -", rotation=45, rotation_mode="anchor",
         va="bottom", ha="center")
plt.savefig("edit_this_with_inkscape.svg")

This is the output of the code. You should be able to edit the y-label and a 45° rotated text in inkscape with center alignment now:
https://gist.github.com/raw/3351009/381d916bb1184135a9dabc2c963d4aff91ee9291/edit_this_with_inkscape.svg

@jenshnielsen

This comment has been minimized.

Show comment Hide comment
@jenshnielsen

jenshnielsen Oct 24, 2012

Owner

Finally I a had a chance to test this out. The new behavior of the svg is a huge improvement and it is really cool
to be able to change the text directly within a latex document and having the text staying nicely aligned in the plot.

Owner

jenshnielsen commented Oct 24, 2012

Finally I a had a chance to test this out. The new behavior of the svg is a huge improvement and it is really cool
to be able to change the text directly within a latex document and having the text staying nicely aligned in the plot.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Nov 14, 2012

Contributor

Merge as a new feature to master?

Contributor

pwuertz commented Nov 14, 2012

Merge as a new feature to master?

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Nov 27, 2012

Contributor

@mdboom , @leejjoon Is this good to merge?

Contributor

pwuertz commented Nov 27, 2012

@mdboom , @leejjoon Is this good to merge?

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Nov 27, 2012

Owner

I think this is cool enough for a "What's New" entry and probably should go in the CHANGELOG as well. The existing SVG tests should be sufficient to test this. Other than that, looks good to me.

Owner

mdboom commented Nov 27, 2012

I think this is cool enough for a "What's New" entry and probably should go in the CHANGELOG as well. The existing SVG tests should be sufficient to test this. Other than that, looks good to me.

@pwuertz

This comment has been minimized.

Show comment Hide comment
@pwuertz

pwuertz Nov 28, 2012

Contributor

I applied the suggested changes. The travis setup for python3 seems to be broken at the moment, local testing succeeded however.

Contributor

pwuertz commented Nov 28, 2012

I applied the suggested changes. The travis setup for python3 seems to be broken at the moment, local testing succeeded however.

@jenshnielsen

This comment has been minimized.

Show comment Hide comment
@jenshnielsen

jenshnielsen Nov 28, 2012

Owner

Cool. The Travis issue looks like this one numpy/numpy#2761

Owner

jenshnielsen commented Nov 28, 2012

Cool. The Travis issue looks like this one numpy/numpy#2761

dmcdougall added a commit that referenced this pull request Nov 30, 2012

Merge pull request #1081 from pwuertz/draw_text_with_mtext
Propagate mpl.text.Text instances to the backends and fix documentation

@dmcdougall dmcdougall merged commit 4ef9115 into matplotlib:master Nov 30, 2012

1 check failed

default The Travis build failed
Details

pwuertz added a commit that referenced this pull request Dec 10, 2012

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