Skip to content

Convert TkAgg utilities to pybind11 #26992

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

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 4, 2023

PR summary

Another step towards finishing #23846. Once again, this depends on the Meson port.

I moved the array handling parts into pybind11 code instead of Python, which seemed to fix some kind of reference counting bug across the callback boundaries.

PR checklist

@oscargus
Copy link
Member

oscargus commented Oct 4, 2023

Blitting on Windows seems to have broken (which you have/will realize).

@ksunden
Copy link
Member

ksunden commented Oct 4, 2023

Hmmm... that test has been flaky at times, but failing 4 times in a pr related to tk is suspicious

@QuLogic
Copy link
Member Author

QuLogic commented Oct 4, 2023

Ah, that's because that test is using the internal TkAgg API, which I simplified using the pybind11 array API, but I missed the test and need to update it.

@QuLogic QuLogic force-pushed the tk-pybind11 branch 2 times, most recently from 0889b76 to 199104d Compare October 4, 2023 21:19
@QuLogic QuLogic marked this pull request as ready for review October 4, 2023 21:22
@QuLogic QuLogic force-pushed the tk-pybind11 branch 4 times, most recently from f05289a to 891989b Compare October 11, 2023 08:00
@story645
Copy link
Member

While making changes here, would it be possible to change WM_DPICHANGED to WM_USERCHANGED since meson is always complaining about it?

@QuLogic
Copy link
Member Author

QuLogic commented Oct 12, 2023

I don't see any warnings about that; it wouldn't have anything to do with Meson anyway.

@story645
Copy link
Member

story645 commented Oct 12, 2023

I don't see any warnings about that; it wouldn't have anything to do with Meson anyway.

I haven't been able to build in weeks, and I'm getting:

meson traceback
(mpl-dev) C:\Users\story\Projects\matplotlib>python -m pip install --no-build-isolation --config-settings=editable-verbose=true --editable .
Obtaining file:///C:/Users/story/Projects/matplotlib
  Checking if build backend supports build_editable ... done
  Preparing editable metadata (pyproject.toml) ... error
  error: subprocess-exited-with-error

  × Preparing editable metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [88 lines of output]
      + meson setup --reconfigure C:\Users\story\Projects\matplotlib C:\Users\story\Projects\matplotlib\build\cp311 -Dbuildtype=release -Db_ndebug=if-release -Db_vscrt=md --native-file=C:\Users\story\Projects\matplotlib\build\cp311\meson-python-native-file.ini
      Cleaning... 0 files.
      The Meson build system
      Version: 1.2.2
      Source dir: C:\Users\story\Projects\matplotlib
      Build dir: C:\Users\story\Projects\matplotlib\build\cp311
      Build type: native build
      Project name: matplotlib
      Project version: 3.9.0.dev0
      C compiler for the host machine: cc (gcc 7.2.0 "cc (Rev1, Built by MSYS2 project) 7.2.0")
      C linker for the host machine: cc ld.bfd 2.29.1
      C++ compiler for the host machine: c++ (gcc 7.2.0 "c++ (Rev1, Built by MSYS2 project) 7.2.0")
      C++ linker for the host machine: c++ ld.bfd 2.29.1
      Host machine cpu family: x86_64
      Host machine cpu: x86_64
      Program python found: YES (C:\Users\story\miniconda3\envs\mpl-dev\python.exe)
      Dependency pybind11 found: YES 2.11.1 (cached)

      Executing subproject freetype-2.6.1

      freetype-2.6.1| Project name: freetype2
      freetype-2.6.1| Project version: 2.6.1
      freetype-2.6.1| C compiler for the host machine: cc (gcc 7.2.0 "cc (Rev1, Built by MSYS2 project) 7.2.0")
      freetype-2.6.1| C linker for the host machine: cc ld.bfd 2.29.1
      freetype-2.6.1| Configuring ftconfig.h using configuration
      freetype-2.6.1| Configuring ftoption.h using configuration
      freetype-2.6.1| Build targets in project: 3
      freetype-2.6.1| Subproject freetype-2.6.1 finished.


      Executing subproject qhull

      qhull| Project name: qhull
      qhull| Project version: 8.0.2
      qhull| C compiler for the host machine: cc (gcc 7.2.0 "cc (Rev1, Built by MSYS2 project) 7.2.0")
      qhull| C linker for the host machine: cc ld.bfd 2.29.1
      qhull| Build targets in project: 4
      qhull| Subproject qhull finished.

      Library dl found: NO
      Library comctl32 found: YES
      Library ole32 found: YES
      Library psapi found: YES
      Library shell32 found: YES
      Library user32 found: YES
      Configuring _version.py using configuration
      Program C:/Users/story/Projects/matplotlib/tools/generate_matplotlibrc.py found: YES (python C:/Users/story/Projects/matplotlib/tools/generate_matplotlibrc.py)
      Build targets in project: 14

      matplotlib 3.9.0.dev0

        Subprojects
          freetype-2.6.1: YES
          qhull         : YES

        User defined options
          Native files  : C:\Users\story\Projects\matplotlib\build\cp311\meson-python-native-file.ini
          buildtype     : release
          b_ndebug      : if-release
          b_vscrt       : md

      Found ninja.EXE-1.11.1 at C:\Users\story\miniconda3\envs\mpl-dev\Library\bin\ninja.EXE
      + meson compile
      [1/7] Compiling C++ object src/_tkagg.cp311-win_amd64.pyd.p/_tkagg.cpp.obj
      FAILED: src/_tkagg.cp311-win_amd64.pyd.p/_tkagg.cpp.obj
      "c++" "-Isrc\_tkagg.cp311-win_amd64.pyd.p" "-Isrc" "-I..\..\src" "-I..\..\..\..\miniconda3\envs\mpl-dev\Lib\site-packages\numpy\core\include" "-I..\..\extern\agg24-svn\include" "-IC:\Users\story\miniconda3\envs\mpl-dev\Include" "-fvisibility=hidden" "-fvisibility-inlines-hidden" "-flto=8" "-fdiagnostics-color=always" "-DNDEBUG" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-std=c++17" "-O3" "-DMS_WIN64=" "-DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION" "-D__STDC_FORMAT_MACROS=1" "-DPY_ARRAY_UNIQUE_SYMBOL=MPL__tkagg_ARRAY_API" -MD -MQ src/_tkagg.cp311-win_amd64.pyd.p/_tkagg.cpp.obj -MF "src\_tkagg.cp311-win_amd64.pyd.p\_tkagg.cpp.obj.d" -o src/_tkagg.cp311-win_amd64.pyd.p/_tkagg.cpp.obj "-c" ../../src/_tkagg.cpp
      ../../src/_tkagg.cpp: In function 'LRESULT DpiSubclassProc(HWND, UINT, WPARAM, LPARAM, UINT_PTR, DWORD_PTR)':
      ../../src/_tkagg.cpp:133:10: error: 'WM_DPICHANGED' was not declared in this scope

           case WM_DPICHANGED:

                ^~~~~~~~~~~~~

      ../../src/_tkagg.cpp:133:10: note: suggested alternative: 'WM_USERCHANGED'

           case WM_DPICHANGED:

                ^~~~~~~~~~~~~

                WM_USERCHANGED

      [2/7] Compiling C object src/_c_internal_utils.cp311-win_amd64.pyd.p/_c_internal_utils.c.obj
      [3/7] Linking target src/_ttconv.cp311-win_amd64.pyd
      [4/7] Linking target src/_tri.cp311-win_amd64.pyd
      [5/7] Linking target src/_image.cp311-win_amd64.pyd
      ninja: build stopped: subcommand failed.
      INFO: autodetecting backend as ninja
      INFO: calculating backend command to run: C:\Users\story\miniconda3\envs\mpl-dev\Library\bin\ninja.EXE
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

Though granted when I fix that error, I get something like _SetProcessDPIAware() not found, so maybe it's just my SDK version is too new (been trying combinations of many things)

@QuLogic
Copy link
Member Author

QuLogic commented Oct 12, 2023

      C compiler for the host machine: cc (gcc 7.2.0 "cc (Rev1, Built by MSYS2 project) 7.2.0")

Ah, do you really intend to use MSYS2 over MSVC? Since you were talking about SDKs, I think you've been intending to use MSVC and never updated MSYS2. You should probably update mingw-w64-x86_64-headers if you want to keep using MSYS2, or force the use of MSVC, or remove MSYS2 if you don't need it.

@story645
Copy link
Member

Ah, do you really intend to use MSYS2 over MSVC?

No, but I have it installed for gitbash 🤦‍♀️ will follow those instructions, thanks!

@QuLogic
Copy link
Member Author

QuLogic commented Oct 12, 2023

Git logs say you would need v6 of the headers, which are about 5 years old now, so probably should update your msys2 in any case.

@oscargus oscargus merged commit bfff33b into matplotlib:main Nov 20, 2023
@QuLogic QuLogic deleted the tk-pybind11 branch November 21, 2023 06:31
@QuLogic QuLogic mentioned this pull request Nov 21, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants