Scale bar: explicit raster controls + renderer-aware DPI#17
Scale bar: explicit raster controls + renderer-aware DPI#17cvanelteren wants to merge 5 commits intomoss-xyz:mainfrom
Conversation
|
@cvanelteren I appreciate you taking a stab at this - and sorry you had to follow my janky conventions for doing type checking and setting up defaults 😅 next time I'm unemployed I'm hoping to switch to a better type checking set up. I've got a few comments though: First, re: your last commit: I don't want to bypass rasterization for unrotated scale bars. I'm worried about creating inconsistent experiences, and unintentionally making the package more difficult to use later down the road (users would have to implicitly know to bypass rasterisation in these types of edge cases). Second, re: the general approach of modifying the dpi of the temp figure that was created: I'm not sure this is actually solving the issue 🤔 regardless of what number I put in for the Are you sure there isn't something else that Ultraplot is doing during the rendering pipeline that is causing the text to appear fuzzy, while the bar artists render clearly? I saw in your last comment as well that you said you could get the text to render clearly when you used an |
|
The issue is that we use a different font by default ( |
|
I will look around a bit more perhaps I missed something. |
We do set the dpi by default to 1k, and add some stuff to the drawing, but from my testing this does not influence this issue. Is the frame on the matplotlib end in the same space or is there another axis created on which the scalebar is plotted? |
|
Hmmm if by frame you mean figure (?), then yes - the scalebar is created on a separate figure and axis Basically when passing an
This pipeline was what I figured out I needed to do in order to place the artists and then render it, such that I could then load the image back into Pillow and return it for placement with Do you think there could be an issue with my use of |
|
I see. That should not be happening. I will investigate on my end and report back. |
|
Ah ok I think I tracked the issue. The DPI mismatch was a nice clue. In |
3cd220d to
8aba1bb
Compare
|
Note I am also fixing the issue on our end. |
|
merged: Ultraplot/UltraPlot#591 |
| # For the default function mode, dispatch to the Artist class so final | ||
| # rasterization happens at draw-time with the active renderer dpi. | ||
| if draw == True and return_aob == True: | ||
| _ = ax.add_artist( | ||
| ScaleBar( | ||
| style=style, | ||
| location=location, | ||
| bar=bar, | ||
| units=units, | ||
| labels=labels, | ||
| text=text, | ||
| aob=aob, | ||
| zorder=zorder, | ||
| ) | ||
| ) | ||
| return |
There was a problem hiding this comment.
@cvanelteren can you talk me through why this is necessary?
Am I understanding correctly that it is because you want to re-using lines 210-212, but it looks to me like that logic is already repeated in lines 298-302, no?
There was a problem hiding this comment.
You’re right that the old code looked duplicated: one place used renderer.dpi in ScaleBar.draw(), and another defaulted to figure.dpi inside scale_bar(). The reason both existed is that they serve different call contexts (draw-time artist render vs direct function render), but the intent is the same: resolve raster DPI consistently. I refactored that into a shared _resolve_raster_dpi(...) helper so both paths now use the same logic, with renderer DPI preferred when available and figure DPI as fallback, then applying raster_dpi_scale in one place. This keeps behavior unchanged but makes the control flow clearer and easier to reason about.
There was a problem hiding this comment.
That's a brilliant change, thank you.
I'll test the changes on my end tomorrow, including with v2.1 of Ultraplot, but I'm curious - what is the difference between draw-time artist render versus direct function render?
I want to make sure I understand why you dispatch to ScaleBar from within scale_bar(), which feels inelegant to me: the code flow basically becomes scale_bar(draw=True) -> ScaleBar -> scale_bar(draw=False) (which then returns to ScaleBar for final rendering). Why is this a better way to go about it, rather than just allowing scale_bar() to finish the render itself?
(Note: some of this is my fault; the return_aob setting is a bit jank actually, definitely something I would look to fix in the next version)
There was a problem hiding this comment.
If scale_bar() “finishes” immediately, it rasterizes earlier using figure-time context, not guaranteed final render context. For this module, that matters because it explicitly rasterizes to an image (_render_as_image + OffsetImage) at lines 503-524. The chain right now, however, is a bit inelegant but it narrows the construction path plan and renderer-correct output. It would be good to split this into explicit apis, but that I leave that up to you.
There was a problem hiding this comment.
Ah wait, does that mean if a user initialises the figure with a DPI of 150, but then decides to render it via savefig or something with a DPI of 300, it will automatically upscale it to a DPI of 300?
If so that actually solves a problem that was identified in an issue earlier: #7
I would love to do a better job of splitting out these portions of the API in the next version - and it seems like I should do so by simply inverting my call stack, so that scale_bar() actually calls ScaleBar and immediately attaches it to the passed ax, as you do here, and then I have a separate function that handles the construction of the scale bar itself (called, in turn, by ScaleBar)!
There was a problem hiding this comment.
That sounds like a good idea I think.
|
@cvanelteren I think we're getting close, thank you for your help so far I've left one comment specific to some lines of code you've added. I also see you removed Finally... how much of this PR even necessary anymore, or were the only fixes that needed to happen on the Ultraplot side of things? I feel like the Comparing the original package version to this branch, I definitely see a difference in how things are rendered, but I'm only on Ultraplot |
The extra stuff are nices to haves but not essential. I think it would still be good to include. The defaults can be left the same as you had them but it prevents potential leakage issues like we introduced on our end (and potential other packages that have a similar issue).
The changes are on main, haven't drafted a release yet for the fix (so it's not available through pypi yet). Quickest would be to install the git directly (or clone and run |
|
The dpi fix is live on 2.1 now. Available on pypi and conda-forge. |
|
I did a bit of initial testing today - but I'll do the final tests and hopefully close out this PR tomorrow! |
|
No rush ;-) |





Summary: Make scale bar raster rendering quality explicit and less sensitive to external rc/style state. Changes: set OffsetImage kwargs explicitly in scale_bar (interpolation default none, dpi_cor default True, resample default False); add bar-level raster controls (raster_dpi and raster_dpi_scale); in ScaleBar.draw(renderer, ...) default raster_dpi to renderer.dpi when not set so exports stay sharp when savefig dpi differs from figure construction dpi; add validation and defaults for these keys. Why: the existing path rasterizes to figure dpi and can blur when save-time dpi differs or when interpolation is inherited from global rc/style.