dpi-related positioning errors in Agg savefig

Development matplotlib-devel
#1

The attached script, run on svn mpl, illustrates a positioning problem: note that the subplot titles are badly positioned in the 50 dpi version. I believe that changing from 150 to 50 dpi should yield a perfect scaling of everything in the plot, but it doesn't. There is a similar problem with a quiver key positioned outside the axes frame, but I don't have a trivial example yet; I am hoping that whatever solves the title positioning problem will take care of that also. If not, I will make a simple example and we can attack it separately.

I have made a first attempt to figure out the cause of the positioning problem, and I have failed; I hope someone else will find it easier to track down. I also find that following the chain of events involved in savefig, and following the dpi setting, is rather difficult, and I wonder whether it might be possible to clarify and simplify anything.

In any case, the dpi-related positioning bug (or bugs) is a significant problem for me now.

Eric

dpi.py (310 Bytes)

#2

There were two problems with title positioning. The first was that
the title offset transformation was hardcoded in pixels and not dpi,
and the second was that is was not getting notifed when the figure dpi
was changed. I changed the offset to read:

self.titleOffsetTrans = mtransforms.Affine2D().translate(
            0.0, 5.0*self.figure.dpi/72.)

and added a callbacks registry to the figure instance so that
observers could be notified when dpi was changed (dpi used to be a
lazy value on the maintenance branch but is a plain-ol-value on the
trunk).

     def on_dpi_change(fig):
            self.titleOffsetTrans.clear().translate(
                0.0, 5.0*fig.dpi/72.)

        self.figure.callbacks.connect('dpi_changed', on_dpi_change)

It looks like the problem in the quiver code is that the "labelsep"
property reads the dpi when the QuiverKey is intiitalized but does not
get notified on dpi change. I added a callback there too so you
should test to see if this helps. The dpi setting is also referenced
in the _set_transform method. Since you are more familiar with the
quiver code that I am, and this hint may point you in the right
direction, I'll let you take a look at that and see if a callback is
needed.

On a related note, one trick I use when debugging text layout problems
is to turn on the "bbox". If the bbox is right but the text is in the
wrong place, you know it is in the layout. If the bbox is in the
wrong place, you know the problem is in the font metrics:

boxprops = dict(facecolor='red')
ax1.set_title('Top Plot', bbox=boxprops)

JDH

···

On Sat, Apr 26, 2008 at 2:04 PM, Eric Firing <efiring@...229...> wrote:

The attached script, run on svn mpl, illustrates a positioning problem: note
that the subplot titles are badly positioned in the 50 dpi version. I
believe that changing from 150 to 50 dpi should yield a perfect scaling of
everything in the plot, but it doesn't. There is a similar problem with a
quiver key positioned outside the axes frame, but I don't have a trivial
example yet; I am hoping that whatever solves the title positioning problem
will take care of that also. If not, I will make a simple example and we
can attack it separately.

#3

John,

Thank you very much--that's a big help. Some time today or tomorrow I will try to track down any remaining quiver problems. I suspect similar dpi-related problems may lurk elsewhere as well.

Those lazy values worked pretty well!

One of the things that puzzles me is the following method of FigureCanvasAgg in backend_agg:

     def print_png(self, filename_or_obj, *args, **kwargs):
         FigureCanvasAgg.draw(self)
         renderer = self.get_renderer()
         original_dpi = renderer.dpi
         renderer.dpi = self.figure.dpi
         if type(filename_or_obj) in (str, unicode):
             filename_or_obj = open(filename_or_obj, 'w')
         self.get_renderer()._renderer.write_png(filename_or_obj, self.figure.dpi)
         renderer.dpi = original_dpi

The FigureCanvasAgg.draw(self) command gets turned into self.figure.draw(self.renderer), which is executed *before* the renderer.dpi gets set to self.figure.dpi, so it seems like there is the possibility of an inconsistency. I suspect there is no problem in practice, but it is confusing.

Now that I look again, it looks like what is happening is that the get_renderer call in FigureCanvasAgg.draw is setting the renderer dpi to the figure dpi, as well as setting self.renderer, in which case most of the code in print_png seems to be superfluous.

Mostly unrelated question: is there any point in keeping backend_agg2.py, or at least keeping it in the backends directory? Maybe it is time to move that and backend_emf.py (and maybe others) to some sort of cold storage, in case someone is inspired later to resurrect them.

Eric

John Hunter wrote:

···

On Sat, Apr 26, 2008 at 2:04 PM, Eric Firing <efiring@...229...> wrote:

The attached script, run on svn mpl, illustrates a positioning problem: note
that the subplot titles are badly positioned in the 50 dpi version. I
believe that changing from 150 to 50 dpi should yield a perfect scaling of
everything in the plot, but it doesn't. There is a similar problem with a
quiver key positioned outside the axes frame, but I don't have a trivial
example yet; I am hoping that whatever solves the title positioning problem
will take care of that also. If not, I will make a simple example and we
can attack it separately.

There were two problems with title positioning. The first was that
the title offset transformation was hardcoded in pixels and not dpi,
and the second was that is was not getting notifed when the figure dpi
was changed. I changed the offset to read:

self.titleOffsetTrans = mtransforms.Affine2D().translate(
            0.0, 5.0*self.figure.dpi/72.)

and added a callbacks registry to the figure instance so that
observers could be notified when dpi was changed (dpi used to be a
lazy value on the maintenance branch but is a plain-ol-value on the
trunk).

     def on_dpi_change(fig):
            self.titleOffsetTrans.clear().translate(
                0.0, 5.0*fig.dpi/72.)

        self.figure.callbacks.connect('dpi_changed', on_dpi_change)

It looks like the problem in the quiver code is that the "labelsep"
property reads the dpi when the QuiverKey is intiitalized but does not
get notified on dpi change. I added a callback there too so you
should test to see if this helps. The dpi setting is also referenced
in the _set_transform method. Since you are more familiar with the
quiver code that I am, and this hint may point you in the right
direction, I'll let you take a look at that and see if a callback is
needed.

On a related note, one trick I use when debugging text layout problems
is to turn on the "bbox". If the bbox is right but the text is in the
wrong place, you know it is in the layout. If the bbox is in the
wrong place, you know the problem is in the font metrics:

boxprops = dict(facecolor='red')
ax1.set_title('Top Plot', bbox=boxprops)

JDH

#4

John,

I made some more changes in quiver, and I think they will ensure that dpi changes are handled correctly as far as arrows are concerned, both in the plot and in the key.

There is still a problem with the key label, and it can be seen clearly in the attached slight modification of quiver_demo.py. There are actually two visible problems:

1) The hline generated by frac is too long with dpi=50.

2) The positioning and sizing of the key bbox are also dpi-dependent, as I verified by using your bbox-display trick.

I went hunting through text.py, mathtext.py, backend_bases, and backend_agg without figuring out where the problem is. The right dpi seems to be getting used at mathtext parsing time. Maybe it is not actually getting propagated all the way into the code that gets the font metrics. I did not try to trace it that far.

Along the way, I found that in backend_bases, print_figure was doing what appears to be a completely unnecessary draw operation after restoring the original dpi, so I commented that out. This speeds up backend_driver.py agg by a few percent.

Eric

John Hunter wrote:

quiver_print.py (2.37 KB)

···

On Sat, Apr 26, 2008 at 2:04 PM, Eric Firing <efiring@...229...> wrote:

The attached script, run on svn mpl, illustrates a positioning problem: note
that the subplot titles are badly positioned in the 50 dpi version. I
believe that changing from 150 to 50 dpi should yield a perfect scaling of
everything in the plot, but it doesn't. There is a similar problem with a
quiver key positioned outside the axes frame, but I don't have a trivial
example yet; I am hoping that whatever solves the title positioning problem
will take care of that also. If not, I will make a simple example and we
can attack it separately.

There were two problems with title positioning. The first was that
the title offset transformation was hardcoded in pixels and not dpi,
and the second was that is was not getting notifed when the figure dpi
was changed. I changed the offset to read:

self.titleOffsetTrans = mtransforms.Affine2D().translate(
            0.0, 5.0*self.figure.dpi/72.)

and added a callbacks registry to the figure instance so that
observers could be notified when dpi was changed (dpi used to be a
lazy value on the maintenance branch but is a plain-ol-value on the
trunk).

     def on_dpi_change(fig):
            self.titleOffsetTrans.clear().translate(
                0.0, 5.0*fig.dpi/72.)

        self.figure.callbacks.connect('dpi_changed', on_dpi_change)

It looks like the problem in the quiver code is that the "labelsep"
property reads the dpi when the QuiverKey is intiitalized but does not
get notified on dpi change. I added a callback there too so you
should test to see if this helps. The dpi setting is also referenced
in the _set_transform method. Since you are more familiar with the
quiver code that I am, and this hint may point you in the right
direction, I'll let you take a look at that and see if a callback is
needed.

On a related note, one trick I use when debugging text layout problems
is to turn on the "bbox". If the bbox is right but the text is in the
wrong place, you know it is in the layout. If the bbox is in the
wrong place, you know the problem is in the font metrics:

boxprops = dict(facecolor='red')
ax1.set_title('Top Plot', bbox=boxprops)

JDH

#5

Eric Firing wrote:

John,

I made some more changes in quiver, and I think they will ensure that dpi changes are handled correctly as far as arrows are concerned, both in the plot and in the key.

There is still a problem with the key label, and it can be seen clearly in the attached slight modification of quiver_demo.py. There are actually two visible problems:

1) The hline generated by frac is too long with dpi=50.

2) The positioning and sizing of the key bbox are also dpi-dependent, as I verified by using your bbox-display trick.

The plot thickens. The postscript backend is completely confused by the quiver plot. Try the attached example.

pdf seems OK.

Eric

q1.py (535 Bytes)

#6

1) The hline generated by frac is too long with dpi=50.

The problem with the hline appears to be that the width of the line in
the frac has a component that depends on the thickness

    thickness = state.font_output.get_underline_thickness(
            state.font, state.fontsize, state.dpi)
    width = max(num.width, den.width) + thickness * 10.

and get_underline_thickness here is defaulting to 1.0 because of this
max call in get_underline_thickness

     return max(1.0, cached_font.font.underline_thickness / 64.0 /
fontsize * 10.0)

Michael -- what is the role of these 10.0 scale factors, which is
showing up in both the frac call and the get_underline_thickness? Is
this something from Knuth, or is it a fudge factor that gets it mostly
right? I suspect we will need something like

    width = max(num.width, den.width) + thickness * 10. * state.dpi/72.

but with this change the frac bar looks a little too wide -- which is
why I'm wondering if there is something magic about 10. or if another
number might serve better.

Along the way, I found that in backend_bases, print_figure was doing what
appears to be a completely unnecessary draw operation after restoring the
original dpi, so I commented that out. This speeds up backend_driver.py agg
by a few percent.

I think you are right here -- because we are creating a new
FigureCanvas and hence a new renderer in the switch_backend call we do
not need to redraw the original figure with the original facecolor,
edgecolor and dpi params. In older versions of mpl, if memory serves
correctly, calling savefig would reuse the same renderer so after the
print to hardcopy we would redraw so the figure in the GUI window
would look right. But I agree this looks superfluous in light of the
current code which is creating a new canvas with each savefig.

JDH

···

On Sun, Apr 27, 2008 at 3:14 AM, Eric Firing <efiring@...229...> wrote:

#7

Argg, that was a subtle one -- there was a bug in backend ps which was
exposed only if you are using a path collection with offsets,
transoffset and clippath and cliprect both None. In that scenario,
the translate in the bind def function was not getting wrapped in a
gsave/grestore pair, which was causing the axes to be drawn in the
wrong place because the translate from the offset remained in effect.
I tried following the draw_markers logic and putting the
gsave/grestore in the ps_cmd, eg in draw_markers Michael writes ijn a
comment:

        ps_cmd = ['/o {', 'gsave', 'newpath', 'translate'] # dont want
the translate to be global

so clearly he was bumping up against the same problem with markers.
For some reason, trying the same thing in the path_collection was not
working for me, so I resorted to the somewhat hackish approach of
forcing _draw_ps to wrap a gsave/grestore if it wasn't getting one
from the cliprect or clippath:

        needwrap = not (clippath or cliprect)
        if needwrap:
            # we need to make sure that there is at least 1
            # save/grestore around each ps write so we'll force it if
            # we're not getting one from the cliprecot or clippath.
            # hackish, yes
            write('gsave\n')

and then

        if needwrap:
            write('grestore\n')

I think there will be cleaner way, but it will need some fresh
eyeballs tomorrow and this will provide a temporary fix.

JDH

···

On Sun, Apr 27, 2008 at 12:56 PM, Eric Firing <efiring@...229...> wrote:

The plot thickens. The postscript backend is completely confused by the
quiver plot. Try the attached example.

#8

One thing I failed to make clear: the artist with said properties is
quiver.QuiverKey.vector

JDH

···

On Sun, Apr 27, 2008 at 8:28 PM, John Hunter <jdh2358@...149...> wrote:

Argg, that was a subtle one -- there was a bug in backend ps which was
exposed only if you are using a path collection with offsets,
transoffset and clippath and cliprect both None. In that scenario,
the translate in the bind def function was not getting wrapped in a

#9

John Hunter wrote:

The plot thickens. The postscript backend is completely confused by the
quiver plot. Try the attached example.

Argg, that was a subtle one -- there was a bug in backend ps which was
exposed only if you are using a path collection with offsets,
transoffset and clippath and cliprect both None. In that scenario,
the translate in the bind def function was not getting wrapped in a
gsave/grestore pair, which was causing the axes to be drawn in the
wrong place because the translate from the offset remained in effect.
I tried following the draw_markers logic and putting the
gsave/grestore in the ps_cmd, eg in draw_markers Michael writes ijn a
comment:

        ps_cmd = ['/o {', 'gsave', 'newpath', 'translate'] # dont want
the translate to be global

so clearly he was bumping up against the same problem with markers.
For some reason, trying the same thing in the path_collection was not
working for me, so I resorted to the somewhat hackish approach of
forcing _draw_ps to wrap a gsave/grestore if it wasn't getting one
from the cliprect or clippath:

        needwrap = not (clippath or cliprect)
        if needwrap:
            # we need to make sure that there is at least 1
            # save/grestore around each ps write so we'll force it if
            # we're not getting one from the cliprecot or clippath.
            # hackish, yes
            write('gsave\n')

and then

        if needwrap:
            write('grestore\n')

I think there will be cleaner way, but it will need some fresh
eyeballs tomorrow and this will provide a temporary fix.

I went ahead and committed an alternative fix that I think is cleaner, and greatly reduces the number of needless gsave/grestore pairs. Your version of the method is still there, with a mangled name, for easy reference and testing until we are sure which way to go.

Sorry for whatever duplicated effort there has been. I did not intend to spend a fair chunk of the day working on this, but I let myself get sucked in. It was a challenge.

As mentioned in a message a few minutes ago that I don't think went to the list, my gs interpreter is choking on the apostrophe (single quote) character, as in title or label strings. I can't find any reason why this should cause trouble--maybe it is a particular version or configuration of gs.

Eric

···

On Sun, Apr 27, 2008 at 12:56 PM, Eric Firing <efiring@...229...> wrote:

#10

John Hunter wrote:

1) The hline generated by frac is too long with dpi=50.
    
The problem with the hline appears to be that the width of the line in
the frac has a component that depends on the thickness

    thickness = state.font_output.get_underline_thickness(
            state.font, state.fontsize, state.dpi)
    width = max(num.width, den.width) + thickness * 10.

and get_underline_thickness here is defaulting to 1.0 because of this
max call in get_underline_thickness

     return max(1.0, cached_font.font.underline_thickness / 64.0 /
fontsize * 10.0)
  

I believe that this is where the dpi needs to be factored in. Ft2font (well, freetype, really) seems to scale metrics for individual glyphs by dpi, but not the font-global metrics. I had wrongly assumed they were all scaled by dpi. So this is a subtler instance of the recent x-height bug that caused sub/superscripts to be incorrect at different dpi's.

Michael -- what is the role of these 10.0 scale factors, which is
showing up in both the frac call and the get_underline_thickness? Is
this something from Knuth, or is it a fudge factor that gets it mostly
right? I suspect we will need something like

    width = max(num.width, den.width) + thickness * 10. * state.dpi/72.
  

This 10.0 is from Knuth. It is the amount of overhang of the fraction beam, and is meant to be relative to the thickness of the beam itself. The other 10.0 (in get_underline_thickness) is my own experimental fudge to deal with the peculiarities of how underline thickness is stored in Truetype fonts. (Remember "pure" TeX uses a completely different font metric standard, so some mapping from TrueType/freetype etc. had to be fudged on.)

What's causing the fraction bar to be too wide at lower dpi is the minimum underline thickness of 1.0 enforced in get_underline_thickness. I think that was a mistake -- enforcing 1.0 pixel lines should happen closer to the backend level -- so I've removed the max(1.0, ...) there.

···

On Sun, Apr 27, 2008 at 3:14 AM, Eric Firing <efiring@...229...> wrote:
but with this change the frac bar looks a little too wide -- which is
why I'm wondering if there is something magic about 10. or if another
number might serve better.
  
--
Michael Droettboom
Science Software Branch
Operations and Engineering Division
Space Telescope Science Institute
Operated by AURA for NASA