Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for High DPI displays to wxAgg backend #26710

Merged
merged 3 commits into from
Mar 27, 2024

Conversation

jmoraleda
Copy link
Contributor

@jmoraleda jmoraleda commented Sep 6, 2023

Summary

This PR adds support to the wxAgg backend to corrertly render plots in high DPI displays in all three platforms that the wx toolkit supports: Linux, MACOS and Windows.

This PR closes issue #7720

Background

This PR adds support for high DPI displays using the underlying toolkit support for high DPI displays: For information of wxPython support for High DPI displays, see https://docs.wxpython.org/high_dpi_overview.html

The wxPython toolkit is built on top of of the C++ wxWidgets toolkit. For a more detailed explanation of support for high DPI displays in the wx ecosystem see the WxWidgets explanation: https://docs.wxwidgets.org/3.2/overview_high_dpi.html

Additional details

This PR correctly scales plots in high DPI displays, including figure size, font size and marker and line width. This PR also includes reading toolbar icons from svg instead of png to make them sharp at all dpi's. I remark this last feature comes with the caveat that automatically changing icon colors for dark themes is not done. [The limitation mentioned in the previous sentence has been removed in the current version of this PR. See discussion below for details. I am crossing out the sentence and adding this note, instead of removing the sentece so the discussion still makes sense]

Additional testing

The variety of use cases and features of matplotlib is very large. I welcome additional testing in all platforms as it is conceivable that there may be features that I have not tested and that I have not added support for. Thank you.

Comment on lines 1057 to 1075
pilimg = PIL.Image.open(cbook._get_data_path("images", name))
# ensure RGBA as wx BitMap expects RGBA format
image = np.array(pilimg.convert("RGBA"))
try:
dark = wx.SystemSettings.GetAppearance().IsDark()
except AttributeError: # wxpython < 4.1
# copied from wx's IsUsingDarkBackground / GetLuminance.
bg = wx.SystemSettings.GetColour(wx.SYS_COLOUR_WINDOW)
fg = wx.SystemSettings.GetColour(wx.SYS_COLOUR_WINDOWTEXT)
# See wx.Colour.GetLuminance.
bg_lum = (.299 * bg.red + .587 * bg.green + .114 * bg.blue) / 255
fg_lum = (.299 * fg.red + .587 * fg.green + .114 * fg.blue) / 255
dark = fg_lum - bg_lum > .2
if dark:
fg = wx.SystemSettings.GetColour(wx.SYS_COLOUR_WINDOWTEXT)
black_mask = (image[..., :3] == 0).all(axis=-1)
image[black_mask, :3] = (fg.Red(), fg.Green(), fg.Blue())
return wx.Bitmap.FromBufferRGBA(
image.shape[1], image.shape[0], image.tobytes())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this lose the compatibility with dark mode? I never could figure out how to test it, though.

Copy link
Contributor Author

@jmoraleda jmoraleda Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuLogic Thank you for your review. You are correct.

I switched from reading the icons from png format to svg format so they will appear sharp in all DPIs.

As you point out and I mentioned in the additional details, this format switch has the caveat that this "black theme" code doesn't work as it is because PIL has no support for svg. Since I have never seen this code execute in practice and I also had not way to test it, I decided the tradeoff of adding sharp icons and removing this code was acceptable. But others may have other opinions.

If someone who uses and can test the "black theme" functionality in hidpi could suggest how to best add black theme support back while reading the icons in SVG format we could add it as part of this PR. Alternatively, it could also be added later as a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a very quick look, given that we generate the SVG icons ourselves, I think we can guarantee that their color can be switched with a simple string replacement, and thus it should be doable to generate the proper-colored SVG icons on the fly?

Copy link
Contributor Author

@jmoraleda jmoraleda Sep 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @anntzer I just looked into this!

Currently our monochromatic svg icons --which are the ones that need changing for black themes-- do not explicitly specify foreground color, so the foreground always appears black since that is the svg default. The background is explicitly specified as transparent which does not need changing for black themes.

I propose that we submit a separate PR making the foreground explicitly black in all monochromatic icons. I just tested this and it is straightforward: It simply requires adding style="fill:black;" in one spot of each svg file.

If the foreground color is explicitly listed in the svg, then adding support for black theme in backends that use svg icons simply involves doing a string replacement fill:black -> fill:white as you suggest.

If you think that this is a reasonable path, I will submit a PR with the icon changes right away, and restore support for black themes in this PR assuming the foreground is explicitly listed as black in monochromatic svg icons.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer Thank you for your quick reply!

Finished with the changes.

  1. I just restored black theme support in the wxAgg backend in this PR.
  2. I explicitly set the foreground color of svg icons to black in a separate PR: Explicitly set foreground color to black in svg icons #26731

I tested both changes together and they work as expected.

@jmoraleda
Copy link
Contributor Author

I squashed the commits to not pollute history

@jmoraleda
Copy link
Contributor Author

Is there anything I can do to help merge this?

@QuLogic
Copy link
Member

QuLogic commented Nov 25, 2023

I gave this a try on Windows, but it didn't seem to work. According to your links, the application needs to set HiDPI awareness in its manifest. We can't really do that however given that we aren't an application, so I think we might need to abstract out the runtime-setting of the awareness from _tkagg.enable_dpi_awareness (and not the window procedure replacement as wxAgg should handle that bit for us). I can perhaps look into this some next week if you are not able to.

I unfortunately do not have a Linux HiDPI setup at the moment to test.

@jmoraleda
Copy link
Contributor Author

jmoraleda commented Nov 25, 2023

Hello @QuLogic Thank you for spending time on this:

the application needs to set HiDPI

From the MSW documentation a Windows script/application that wants to handle its own scaling --instead of letting the OS do the scaling in a generic way with blurry results-- must let the OS know. This is true regardless of the language the application is written in or any frameworks used. In python this is achieved with the following code:

    import ctypes
    ctypes.windll.shcore.SetProcessDpiAwareness(1)

My understanding is that this code should be part of the end user's application or script, not set by libraries (e.g. matplotlib) or frameworks (e.g. wx) because it is the user who knows if all the libraries and components they use are capable of handling their own scaling. If the end user set's it, wx, matplotlib and other graphical components should read the actual DPI value and handle scaling in a library specific manner. wx already behaves this way. I believe this PR makes matplotlib behave this way when used in conjunction with wx.

If you have a chance, please try adding these lines to your test script and let me know the results. If you encounter any issues I can work with you on them. Thank you.

Additional notes for the record

  • A wx multiplatform application would include the above code inside: if wx.Platform == '__WXMSW__':
  • Using a value of one in the code above sets per-process dpi awareness, which is sufficient if one has a single monitor or all monitors have the same dpi. If the application wants to handle multiple monitors with different dpi's then one can set per monitor dpi awareness instead: One would call SetProcessDpiAwareness(3) per the reference above, and optionally implement a wx callback in each top level window to react the event of the window being dragged to a monitor with a different DPI in a way suitable to the application. This may involve changing the layout of widgets, etc.

@QuLogic
Copy link
Member

QuLogic commented Nov 29, 2023

When Matplotlib is the one managing windows (i.e., through pyplot), then it is the application that should enable HiDPI mode. See the triggers in the Tk backend on the manager for example.

@jmoraleda
Copy link
Contributor Author

jmoraleda commented Nov 29, 2023

When Matplotlib is the one managing windows (i.e., through pyplot), then it is the application that should enable HiDPI mode.

This makes sense to me

See the triggers in the Tk backend on the manager for example.

In _backend_tk.py I find the block of code

self._window_dpi_cbname = ''
if _tkagg.enable_dpi_awareness(window_frame, window.tk.interpaddr()):
    self._window_dpi_cbname = self._window_dpi.trace_add(
         'write', self._update_window_dpi)

This code calls the function defined at _tkagg.cpp which itself does call the MSW library to set awareness to "per-monitor" awareness unconditionally.

To achieve the same functionality in the wx backend It would be trivial to add the following lines to the wx backend:

if wx.Platform == '__WXMSW__':
    import ctypes
    ctypes.windll.shcore.SetProcessDpiAwareness(3)

But I do not think in the wx backend case it makes sense to call these lines unconditionally because the typical use of matplotlib with the wx backend is to have figures embedded in larger wx applications rather than in stand alone windows. So I think we need to let the end user retain control of whether dpi awareness should be handled by their application or the OS. What do you think?

If you agree we could add these lines as a function to the wx backend that the user can invoke if they so choose. Or perhaps better, to avoid redundancy, we could document the functionality and have the user call the original MSW SetProcessDPIAwareness with a value of their choosing.

@jmoraleda
Copy link
Contributor Author

Is there anything I can do to help merge this?

@tianzhuqiao
Copy link

Thanks for the PR. The text/plot looks great on macOS. However, for the following code, the initial plot screen doesn't looks correct, and the toolbar also looks a little weird.

>>> import matplotlib
>>> matplotlib.use('wxAgg')
>>> import matplotlib.pyplot as plt
>>> plt.plot([1,2,3,4,5])
>>> plt.show()

Initial plot screen from above code

After resize the plot window (drag the edge), the axes area looks great (the toolbar area looks same as above)

tianzhuqiao pushed a commit to tianzhuqiao/bsmedit that referenced this pull request Feb 4, 2024
@jmoraleda
Copy link
Contributor Author

Thank you @tianzhuqiao for testing this, and for your feedback. I don't have access to a Mac, but I am going to look into it and try to fix it and, if you don't mind, I will ask you to please test on a Mac. Thank you in advance. I am currently traveling, so it may take me a few weeks to get back to you.

@tianzhuqiao
Copy link

@jmoraleda, sure, no problem. And draw_rubberband may also need to be updated, otherwise, it shows in a wrong location.

def draw_rubberband(self, event, x0, y0, x1, y1):

@tianzhuqiao
Copy link

Thanks. The initial plot and icon size look great on Mac now.
image

@jmoraleda
Copy link
Contributor Author

@tianzhuqiao I believe my last commit fixes the draw_rubberband in all platforms. Thank you for pointing this out!

@rcomer
Copy link
Member

rcomer commented Mar 4, 2024

The tests failed because of a problem with a new version of pytest which has now been yanked. I’m going to close and re-open to restart the CI.

@rcomer rcomer closed this Mar 4, 2024
@rcomer rcomer reopened this Mar 4, 2024
@tianzhuqiao
Copy link

Thanks @jmoraleda , draw_rubberband works on macOS now.

@tacaswell
Copy link
Member

tacaswell commented Mar 6, 2024

But I do not think in the wx backend case it makes sense to call these lines unconditionally because the typical use of matplotlib with the wx backend is to have figures embedded in larger wx applications rather than in stand alone windows.

I am a bit curious what this assertion is based on. My expectation is the opposite in that independent of backend we are going to have many more users of pyplot than who are doing embedding in a large GUI (if for no other reason than the barrier for entry is much lower with pyplot). On the other hand, wx is after tk in the fallback list so it is possible it only very rarely gets auto-selected. However, leaving that aside we do have users of both pyplot + wx and embedding + wx so we need to make this work for both.

So I think we need to let the end user retain control of whether dpi awareness should be handled by their application or the OS. What do you think?

If we go through

@functools.lru_cache(1)
def _create_wxapp():
wxapp = wx.App(False)
wxapp.SetExitOnFrameDelete(True)
cbook._setup_new_guiapp()
return wxapp
then we are definitely creating the App not the user. In that case we should do the helpful thing and enable hi-dpi like the other platforms and backends rather than making users on wx on windows jump through extra hoops.

A similar bit of logic should probably be pushed to IPython in the case that they make the App.

[edited to fix markup]

@jmoraleda
Copy link
Contributor Author

Brief answer

I think my last commit does the right thing in all cases that we have discussed :-). Please test if you have a chance!

Longer answer

@tacaswell Thank you for reviewing this and especially for bringing _create_wxapp to my attention.

I had not paid attention to the fact that this function is only invoked if a wx.App has not previously been created. This opens a beautiful path to address simultaneously the requirements of both constituencies:

  • dpi awareness should be set automatically when matplotlib is creating the application. In paticular, matplotlib code should just work out of the box as expected taking full advantage of monitor resolutions, as @QuLogic pointed out above.
  • dpi awareness should be set by the end-user application when embedding matplotlib figures in a pre-existing wx application so that the user can decide what best suites them.

I just pushed a commit that sets per-monitor dpi awareness inside _create_wxapp thus addressing both requirements.

Now typical matplotlib code such as:

import matplotlib
matplotlib.use('wxAgg')
import matplotlib.pyplot as plt
plt.plot([1,2,3,4,5])
plt.show()

just works out of the box taking advantage of the full monitor resolution as expected. While wx users that embed matplotlib figures in their applications can still set dpi as they wish.

Remark

Per MSW documentation, once a process makes an API call to choose how to handle dpi awareness, future calls will be ignored and the initial setting will not be changed:

Once you set the DPI awareness for a process, it cannot be changed.

(https://learn.microsoft.com/en-us/windows/win32/api/shellscalingapi/ne-shellscalingapi-process_dpi_awareness)

This is why we shouldn't always set per-process dpi awareness when using the wx backend, since there are applications that do not implement support to handle this in their non-matplotlib windows and would thus break if the O/S expected them to.

@jmoraleda
Copy link
Contributor Author

The remaining linter warning is about a line that is too long. That line is a comment containing a url to MS documentation on the topic of setting dpi awareness. I think it is useful to retain it for future reference, but let me know what you think. Thank you.

@jmoraleda
Copy link
Contributor Author

Are there any outstanding issues or questions? Is there anything I can do to help merge this?

@@ -45,6 +43,10 @@ def _create_wxapp():
wxapp = wx.App(False)
wxapp.SetExitOnFrameDelete(True)
cbook._setup_new_guiapp()
if wx.Platform == '__WXMSW__':
# Set per-process DPI awareness. See https://docs.microsoft.com/en-us/windows/win32/api/shellscalingapi/ne-shellscalingapi-process_dpi_awareness
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flake8 will be happy if the long URL is on a line by itself:

Suggested change
# Set per-process DPI awareness. See https://docs.microsoft.com/en-us/windows/win32/api/shellscalingapi/ne-shellscalingapi-process_dpi_awareness
# Set per-process DPI awareness. For more details, see
# https://docs.microsoft.com/en-us/windows/win32/api/shellscalingapi/ne-shellscalingapi-process_dpi_awareness

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Good to know. I ended up removing the link because using the internal function that you suggest in your next comment makes it unnecessary.

Comment on lines 48 to 49
import ctypes
ctypes.windll.shcore.SetProcessDpiAwareness(2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the wrapper from _c_internal_utils as I stated before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QuLogic Thank you for catching that. Fixed.

@tacaswell tacaswell added this to the v3.9.0 milestone Mar 27, 2024
@ksunden ksunden merged commit de11026 into matplotlib:main Mar 27, 2024
40 of 43 checks passed
@QuLogic
Copy link
Member

QuLogic commented Apr 2, 2024

It's a bit unfortunate that AppVeyor was already in a broken state, because that hid that this was failing:

___ test_interactive_backend[toolmanager-MPLBACKEND=wxagg-BACKEND_DEPS=wx] ____
[gw0] win32 -- Python 3.9.19 C:\Miniconda3-x64\envs\mpl-dev\python.exe
env = {'BACKEND_DEPS': 'wx', 'MPLBACKEND': 'wxagg'}, toolbar = 'toolmanager'
    @pytest.mark.parametrize("env", _get_testable_interactive_backends())
    @pytest.mark.parametrize("toolbar", ["toolbar2", "toolmanager"])
    @pytest.mark.flaky(reruns=3)
    def test_interactive_backend(env, toolbar):
        if env["MPLBACKEND"] == "macosx":
            if toolbar == "toolmanager":
                pytest.skip("toolmanager is not implemented for macosx.")
        if env["MPLBACKEND"] == "wx":
            pytest.skip("wx backend is deprecated; tests failed on appveyor")
        try:
>           proc = _run_helper(
                _test_interactive_impl,
                json.dumps({"toolbar": toolbar}),
                timeout=_test_timeout,
                extra_env=env,
            )
C:\projects\matplotlib\lib\matplotlib\tests\test_backends_interactive.py:256: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
C:\projects\matplotlib\lib\matplotlib\testing\__init__.py:126: in subprocess_run_helper
    proc = subprocess_run_for_testing(
C:\projects\matplotlib\lib\matplotlib\testing\__init__.py:94: in subprocess_run_for_testing
    proc = subprocess.run(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
input = None, capture_output = False, timeout = 120, check = True
popenargs = (['C:\\Miniconda3-x64\\envs\\mpl-dev\\python.exe', '-c', "import importlib.util;_spec = importlib.util.spec_from_file_...e_from_spec(_spec);_spec.loader.exec_module(_module);_module._test_interactive_impl()", '{"toolbar": "toolmanager"}'],)
kwargs = {'env': {'7ZIP': '"C:\\Program Files\\7-Zip\\7z.exe"', 'ALLUSERSPROFILE': 'C:\\ProgramData', 'APPDATA': 'C:\\Users\\appveyor\\AppData\\Roaming', 'APPVEYOR': 'True', ...}, 'stderr': -1, 'stdout': -1, 'text': True}
process = <Popen: returncode: 1 args: ['C:\\Miniconda3-x64\\envs\\mpl-dev\\python.exe'...>
stdout = ''
stderr = 'C:\\Miniconda3-x64\\envs\\mpl-dev\\lib\\_collections_abc.py:941: UserWarning: Treat the new Tool classes introduced i... ""Assert failure"" failed at ..\\..\\src\\msw\\toolbar.cpp(963) in wxToolBar::Realize(): invalid tool button bitmap\n'
retcode = 1
    def run(*popenargs,
            input=None, capture_output=False, timeout=None, check=False, **kwargs):
        """Run command with arguments and return a CompletedProcess instance.
    
        The returned instance will have attributes args, returncode, stdout and
        stderr. By default, stdout and stderr are not captured, and those attributes
        will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them.
    
        If check is True and the exit code was non-zero, it raises a
        CalledProcessError. The CalledProcessError object will have the return code
        in the returncode attribute, and output & stderr attributes if those streams
        were captured.
    
        If timeout is given, and the process takes too long, a TimeoutExpired
        exception will be raised.
    
        There is an optional argument "input", allowing you to
        pass bytes or a string to the subprocess's stdin.  If you use this argument
        you may not also use the Popen constructor's "stdin" argument, as
        it will be used internally.
    
        By default, all communication is in bytes, and therefore any "input" should
        be bytes, and the stdout and stderr will be bytes. If in text mode, any
        "input" should be a string, and stdout and stderr will be strings decoded
        according to locale encoding, or by "encoding" if set. Text mode is
        triggered by setting any of text, encoding, errors or universal_newlines.
    
        The other arguments are the same as for the Popen constructor.
        """
        if input is not None:
            if kwargs.get('stdin') is not None:
                raise ValueError('stdin and input arguments may not both be used.')
            kwargs['stdin'] = PIPE
    
        if capture_output:
            if kwargs.get('stdout') is not None or kwargs.get('stderr') is not None:
                raise ValueError('stdout and stderr arguments may not be used '
                                 'with capture_output.')
            kwargs['stdout'] = PIPE
            kwargs['stderr'] = PIPE
    
        with Popen(*popenargs, **kwargs) as process:
            try:
                stdout, stderr = process.communicate(input, timeout=timeout)
            except TimeoutExpired as exc:
                process.kill()
                if _mswindows:
                    # Windows accumulates the output in a single blocking
                    # read() call run on child threads, with the timeout
                    # being done in a join() on those threads.  communicate()
                    # _after_ kill() is required to collect that and add it
                    # to the exception.
                    exc.stdout, exc.stderr = process.communicate()
                else:
                    # POSIX _communicate already populated the output so
                    # far into the TimeoutExpired exception.
                    process.wait()
                raise
            except:  # Including KeyboardInterrupt, communicate handled that.
                process.kill()
                # We don't call process.wait() as .__exit__ does that for us.
                raise
            retcode = process.poll()
            if check and retcode:
>               raise CalledProcessError(retcode, process.args,
                                         output=stdout, stderr=stderr)
E               subprocess.CalledProcessError: Command '['C:\\Miniconda3-x64\\envs\\mpl-dev\\python.exe', '-c', "import importlib.util;_spec = importlib.util.spec_from_file_location('matplotlib.tests.test_backends_interactive', 'C:\\\\projects\\\\matplotlib\\\\lib\\\\matplotlib\\\\tests\\\\test_backends_interactive.py');_module = importlib.util.module_from_spec(_spec);_spec.loader.exec_module(_module);_module._test_interactive_impl()", '{"toolbar": "toolmanager"}']' returned non-zero exit status 1.
C:\Miniconda3-x64\envs\mpl-dev\lib\subprocess.py:528: CalledProcessError
During handling of the above exception, another exception occurred:
env = {'BACKEND_DEPS': 'wx', 'MPLBACKEND': 'wxagg'}, toolbar = 'toolmanager'
    @pytest.mark.parametrize("env", _get_testable_interactive_backends())
    @pytest.mark.parametrize("toolbar", ["toolbar2", "toolmanager"])
    @pytest.mark.flaky(reruns=3)
    def test_interactive_backend(env, toolbar):
        if env["MPLBACKEND"] == "macosx":
            if toolbar == "toolmanager":
                pytest.skip("toolmanager is not implemented for macosx.")
        if env["MPLBACKEND"] == "wx":
            pytest.skip("wx backend is deprecated; tests failed on appveyor")
        try:
            proc = _run_helper(
                _test_interactive_impl,
                json.dumps({"toolbar": toolbar}),
                timeout=_test_timeout,
                extra_env=env,
            )
        except subprocess.CalledProcessError as err:
>           pytest.fail(
                "Subprocess failed to test intended behavior\n"
                + str(err.stderr))
E           Failed: Subprocess failed to test intended behavior
E           C:\Miniconda3-x64\envs\mpl-dev\lib\_collections_abc.py:941: UserWarning: Treat the new Tool classes introduced in v1.5 as experimental for now; the API and rcParam may change in future versions.
E             self[key] = other[key]
E           Traceback (most recent call last):
E             File "<string>", line 1, in <module>
E             File "C:\projects\matplotlib\lib\matplotlib\tests\test_backends_interactive.py", line 182, in _test_interactive_impl
E               plt.figure()
E             File "C:\projects\matplotlib\lib\matplotlib\pyplot.py", line 1005, in figure
E               manager = new_figure_manager(
E             File "C:\projects\matplotlib\lib\matplotlib\pyplot.py", line 528, in new_figure_manager
E               return _get_backend_mod().new_figure_manager(*args, **kwargs)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_bases.py", line 3490, in new_figure_manager
E               return cls.new_figure_manager_given_figure(num, fig)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_bases.py", line 3495, in new_figure_manager_given_figure
E               return cls.FigureCanvas.new_manager(figure, num)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_bases.py", line 1803, in new_manager
E               return cls.manager_class.create_with_canvas(cls, figure, num)
E             File "C:\projects\matplotlib\lib\matplotlib\backends\backend_wx.py", line 967, in create_with_canvas
E               frame = FigureFrameWx(num, figure, canvas_class=canvas_class)
E             File "C:\projects\matplotlib\lib\matplotlib\backends\backend_wx.py", line 909, in __init__
E               manager = FigureManagerWx(self.canvas, num, self)
E             File "C:\projects\matplotlib\lib\matplotlib\backends\backend_wx.py", line 961, in __init__
E               super().__init__(canvas, num)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_bases.py", line 2672, in __init__
E               tools.add_tools_to_container(self.toolbar)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_tools.py", line 1007, in add_tools_to_container
E               container.add_tool(tool, group, position)
E             File "C:\projects\matplotlib\lib\matplotlib\backend_bases.py", line 3330, in add_tool
E               self.add_toolitem(tool.name, group, position,
E             File "C:\projects\matplotlib\lib\matplotlib\backends\backend_wx.py", line 1218, in add_toolitem
E               self.Realize()
E           wx._core.wxAssertionError: C++ assertion ""Assert failure"" failed at ..\..\src\msw\toolbar.cpp(963) in wxToolBar::Realize(): invalid tool button bitmap
C:\projects\matplotlib\lib\matplotlib\tests\test_backends_interactive.py:263: Failed

QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Apr 2, 2024
@jmoraleda
Copy link
Contributor Author

@QuLogic What is the best way of running locally the exact appveyor command that fails?

When I run pytest --pyargs matplotlib.tests I get 4 errors but none is related to the wx backend:

================================= test session starts ==================================
platform linux -- Python 3.11.2, pytest-7.2.1, pluggy-1.0.0+repack
rootdir: /home/jorge/vc/matplotlib, configfile: pyproject.toml
plugins: anyio-3.6.2
collected 9365 items / 4 errors / 1 skipped                                            

======================================== ERRORS ========================================
_________________________ ERROR collecting test_backend_pgf.py _________________________
/usr/lib/python3/dist-packages/matplotlib/tests/test_backend_pgf.py:98: in <module>
    @pytest.mark.skipif(not _has_tex_package('type1ec'), reason='needs type1ec.sty')
/usr/lib/python3/dist-packages/matplotlib/testing/__init__.py:112: in _has_tex_package
    mpl.dviread._find_tex_file(f"{package}.sty")
E   AttributeError: module 'matplotlib.dviread' has no attribute '_find_tex_file'
_______________________ ERROR collecting test_preprocess_data.py _______________________
ImportError while importing test module '/usr/lib/python3/dist-packages/matplotlib/tests/test_preprocess_data.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3/dist-packages/matplotlib/tests/test_preprocess_data.py:9: in <module>
    from matplotlib.testing import subprocess_run_for_testing
E   ImportError: cannot import name 'subprocess_run_for_testing' from 'matplotlib.testing' (/usr/lib/python3/dist-packages/matplotlib/testing/__init__.py)
___________________________ ERROR collecting test_pyplot.py ____________________________
ImportError while importing test module '/usr/lib/python3/dist-packages/matplotlib/tests/test_pyplot.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3/dist-packages/matplotlib/tests/test_pyplot.py:10: in <module>
    from matplotlib.testing import subprocess_run_for_testing
E   ImportError: cannot import name 'subprocess_run_for_testing' from 'matplotlib.testing' (/usr/lib/python3/dist-packages/matplotlib/testing/__init__.py)
__________________________ ERROR collecting test_sphinxext.py __________________________
ImportError while importing test module '/usr/lib/python3/dist-packages/matplotlib/tests/test_sphinxext.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3/dist-packages/matplotlib/tests/test_sphinxext.py:9: in <module>
    from matplotlib.testing import subprocess_run_for_testing
E   ImportError: cannot import name 'subprocess_run_for_testing' from 'matplotlib.testing' (/usr/lib/python3/dist-packages/matplotlib/testing/__init__.py)
=================================== warnings summary ===================================
../../../../usr/lib/python3/dist-packages/mpl_toolkits/mplot3d/axes3d.py:35
  /usr/lib/python3/dist-packages/mpl_toolkits/mplot3d/axes3d.py:35: MatplotlibDeprecationWarning: Importing matplotlib.tri.triangulation was deprecated in Matplotlib 3.7 and will be removed two minor releases later. All functionality is available via the top-level module matplotlib.tri
    from matplotlib.tri.triangulation import Triangulation

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=============================== short test summary info ================================
ERROR test_backend_pgf.py - AttributeError: module 'matplotlib.dviread' has no attribute '_find_tex_file'
ERROR test_preprocess_data.py
ERROR test_pyplot.py
ERROR test_sphinxext.py
!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 4 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!
======================= 1 skipped, 1 warning, 4 errors in 10.79s =======================

@QuLogic
Copy link
Member

QuLogic commented Apr 2, 2024

You should be able to run any script and set plt.rcParams['toolbar'] = 'toolmanager' to reproduce; at least the missing icons were reproducible on Linux that way.

@QuLogic
Copy link
Member

QuLogic commented Apr 2, 2024

Oh also, if you're getting those errors when running pytest, it appears you've not installed Matplotlib correctly, and probably have some old copy in your environment.

@jmoraleda
Copy link
Contributor Author

You should be able to run any script and set plt.rcParams['toolbar'] = 'toolmanager' to reproduce; at least the missing icons were reproducible on Linux that way.

I was able to reproduce the problem of the missing toolmanager icons running examples/user_interfaces/toolmanager_sgskip.py after adding at the top

import matplotlib
matplotlib.use('wxAgg')

but that is fixed now with #28007, which I think we should merge.

But #28007 does not fix the appveyor problem and I don't know how to run tests\test_backends_interactive.py which seems to be the script failing in appveyor.

@jmoraleda
Copy link
Contributor Author

jmoraleda commented Apr 3, 2024

Oh also, if you're getting those errors when running pytest, it appears you've not installed Matplotlib correctly, and probably have some old copy in your environment.

Very possible. My default installation of matplotlib in debian is via apt and the package manager, and my earlier run of the tests was on a patched version of that that includes the latest wx backend code, instead of in a clean installation of all the latest code in a venv. But my point was that no test failed related to wx.

@QuLogic
Copy link
Member

QuLogic commented Apr 3, 2024

But my point was that no test failed related to wx.

No tests ran at all, as it failed to import.

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

Successfully merging this pull request may close these issues.

WxAgg backend draws the wrong size when wxpython app is high DPI aware on Windows
7 participants