Replace quiver dpi callback with reinit-on-dpi-changed. #22807
Conversation
Instead of registering a callback to update itself whenever the dpi changes, do the same thing as every other artist which depends on dpi (e.g. FancyArrowPatch), i.e. update oneself as needed. We can still save duplicate calculations by only doing the init if the figure dpi changed compared to last time. Also drop _new_UV, which is never read.
|
Nice! Thanks for the quick fix. I had just started looking into this as well and was going to change to using "resize_event" on the canvas which is what is really desired I think, but this is nicer :) On that note, I assume you realized that "dpi_changed" isn't used anywhere else in the codebase, so I would vote for deprecating that in another PR too... I think |
|
Not looking at the details, but should these things be done in trans_dpi_acale? |
|
I'm not sure that that would be possible as these are arrows positioned in data space, but whose size is determined by screen space. |
|
We have this problem relatively often, at least with arrows - we want a glyph to have one dimension in data space, but the shape of the arrow heads in physical space. This doesn't seem too hard to do in physical space at draw time. Transfer the anchors to physical space and then draw the arrow and let its transform be physical space (for which we need a better name than trans_dpi_scale) if we do that dpi changes happen at the renderer level and the upper level code doesn't need a callback. |
|
Is this only used for labelsep? Can we place text with an offset transform? |
| @@ -562,18 +525,19 @@ def _init(self): | |||
| """ | |||
| # It seems that there are not enough event notifications | |||
| # available to have this work on an as-needed basis at present. | |||
| if True: # not self._initialized: | |||
| if True: # self._dpi_at_last_init != self.axes.figure.dpi | |||
There was a problem hiding this comment.
Why did you change the comment? why does this code get run unconditionally?
There was a problem hiding this comment.
I don't know why the original check got commented out (and don't really want to investigate right now), I'm just replacing _initialized by its newer moral equivalent.
|
|
||
| def _init(self): | ||
| if True: # not self._initialized: | ||
| if not self.Q._initialized: | ||
| if True: # self._dpi_at_last_init != self.axes.figure.dpi |
There was a problem hiding this comment.
I don't understand why this condition is here if its always true?
|
Agree that deprecating dpi_changed is perhaps a good idea, but this pr is really just aiming for the minimum fix. Ditto for using offset_transform or dpi_scale_trans instead. |
|
@meeseeksdev backport to v3.5.x |
|
Could not push to auto-backport-of-pr-22807-on-v3.5.x due to error, aborting. |
…-on-dpi-changed.
|
@meeseeksdev backport to v3.5.x |
|
Could not push to auto-backport-of-pr-22807-on-v3.5.x due to error, aborting. |
|
meeseeksdev backport to v3.5.x |
…-on-dpi-changed.
…807-on-v3.5.x Backport PR #22807 on branch v3.5.x (Replace quiver dpi callback with reinit-on-dpi-changed.)
Instead of registering a callback to update itself whenever the dpi
changes, do the same thing as every other artist which depends on dpi
(e.g. FancyArrowPatch), i.e. update oneself as needed. We can still
save duplicate calculations by only doing the init if the figure dpi
changed compared to last time.
Also drop _new_UV, which is never read.
Closes #22804.
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).