Simplify and fix dpi handling in tight_bbox#2621
Simplify and fix dpi handling in tight_bbox#2621tacaswell merged 2 commits intomatplotlib:masterfrom pwuertz:fix_2586
Conversation
|
It seems that @leejjoon contributed most of the code related to the bbox feature. May I ask your opinion on this? @mdboom probably as well. Unfortunately the master already advanced to a point where the PR isn't mergable anymore. I would like to wait for your consent for these changes before bringing them up to date again. |
|
👍 for this change. It doesn't merge cleanly (probably the api_changes.rst file) so will need a rebase before merging. @leejjoon, thoughts? |
|
Do we know if these functions are used by anyone outside the library? If they are purely private functions, should they be renamed with leading |
|
You could argue that these functions are "private enough" since they reside in a module a user normally wouldn't import, but we don't really know if someone might be using the |
|
👍 to a privacy MEP if anyone wants to write one. (I'm not volunteering...) |
|
@mdboom, so, thumbs up for this PR too ;) ? |
|
Looks fine to me. @leejjoon probably understands the potential issues, better, though. If he is unavailable for comment before 1.4.0rc1, I say we merge this anyway. |
Simplify and fix dpi handling in tight_bbox
4 participants
Being based on PR #2588, this PR is now able to (again) fix
tight_bbox.adjust_bboxhandling of PGF figures.The problem with that function is its dependence on a figure's dpi, which it naturally takes from the figure instance. This behaviour was implemented in
adjust_bbox_png(). Whenadjust_bbox()is called fromFigureCanvas.print_figure()this dpi value might be incorrect, since backends like svg and pdf use a fixed value of 72dpi and override the figure dpi later when calling theirprint_()methods. The solution was to predict this setting from the file format and use the 72dpi implementation calledadjust_bbox_pdf(). This fails because the mapping from file types to backends is ambiguous.I fixed this by moving all per-backend information from
tight_bbox.pyto the backends. The code duplication inadjust_bbox()is gone. The function instead provides a parameter for setting a custom dpi, which a backend/FigureCanvas announces if necessary.The second commit adds a missing clip rectangle in backend_pgf (reported in #2586) and an appropriate test.