Closed MestreLion closed 1 year ago
MestreLion
commented
1 year ago Speaking of isActive()...
This is a nitpick, but... you've removed the ) -> bool: return type annotation in 552f4797932fb55be4127f5ef54fb41466f366c7:
https://github.com/Kalmat/PyWinCtl/blame/master/src/pywinctl/_pywinctl_linux.py#L734
MestreLion
commented
1 year ago Another idea, if you're uncomfortable with a module-level display instance, is to make it a class attribute:
# optional, but such aliases really helps make the code cleaner
XDisplay: TypeAlias = Xlib.display.Display
XWindow: TypeAlias = Xlib.xobject.drawable.Window
class LinuxWindow(BaseWindow):
display: XDisplay | None = None
root: XWindow | None = None
def __init__(self, ...) -> None:
if self.display is None:
self.init_display(...)
...
@classmethod
def init_display(cls, ...) -> None:
cls.display = Xlib.display.Display(...)
cls.root = ...
...So only the first created Window takes the penalty, and only once, while all others created afterwards benefit from an already instanced display
MestreLion
commented
1 year ago Meanwhile, is it possible for you to release (or at least tag) commit 4b4a9aeb89ab40f436aea7b47dfd24275c3e044d ? This is the last commit after a bunch of improvements and bugfixes since 0.0.42 and right before this deeper rework and ewmh replacement.
Replacing a "core" library is a huge task, specially if you are developing the replacement by yourself (once again, sorry for having paused my ewmh-client project before it is useful to you). It will take time, it might break a few things and have a few rough edges. So until the dust settles I believe a "last release before the revamp" would be awesome!
MestreLion
commented
1 year ago Another idea, if you're uncomfortable with a module-level
displayinstance, is to make it a class attribute:
Actually, considering you have module-level functions in your API, such as getActiveWIndow() itself, I'm afraid the best approach is to create a module-level singleton instance at import time:
import ...
...
XDisplay: TypeAlias = Xlib.display.Display
XWindow: TypeAlias = Xlib.xobject.drawable.Window
...
display: XDisplay = Xlib.display.Display(...)
root: XWindow = display.xxx.yyy.rootUnless you're willing to add a if display is None then: init_display() or similar in every single function of the whole module. Not worth it IMHO.
Btw, this is not very different than what you used to with ewmh: you had a module-level EWMH = ewmh.EWMH() instance singleton that encapsulated the whole thing, but in essence it's the same idea. Actually, you even had DISP = Xlib.display.Display(...) and ROOT = .. that you used to create the EWMH instance. And I believe there's no (good) way to avoid this.
Kalmat
commented
1 year ago First of all, I've seen you already replaced the infamous
ewmhmodule for your own direct Xlib calls. Oh my, that must've been hard! What a huge effort, so congrats!
Translate just what I was using in pywinctl, and without properly design, protect against errors and test, was not so much actually, but thank you!
Second, my apologies for being silent on my own
ewmhreplacement. I had to take another priority project, so I can't continue our amazing discussions and brainstorming for now. Still, all your ideas and comments are properly stored (thanks Github!) and ready to resume as soon as I have more spare time. It might take a while tho...
Do not worry at all! First things, first. To be honest, I thought I had overwhelmed you because I realized I was so excited with the idea of the new module that I entered like an "elephant in a glass shop"!!! HAHAHA!
Any particular reason to change from
python-xlibback toxlib? As explained in here and mentioned in #41 , thexlibPypi package is a non official, unmaintained fork, its last release was more than 5 years ago!python-xlibis the one published by the official repository, is actively maintained, last release was less than 3 months ago,
My fault. I have two repositories to manage about typing (eternal hate) and very likely I overwrote the right version.
So... you're creating a new
Display()instance on every call? That's terribly inefficient! A display instance creates a new connection to an X11 server, and the way Xlib packages does that (find the local unix socket in the filesystem, cache all atoms, etc) makes this a very expensive call, should do preferably only once. The new approach has a huge performance penalty:
My fault again. What I sent you was a very preliminary version, still to be deeply reviewed and tested. I just was anxious to help! These days I have developed a totally new version. Inevitably, I have applied the ideas I was sharing with you. This other module is also work in progress and needs to be improved and tested, but it's much better (I hope). Reading the docs also help! HAHAHA! I'm attaching it so you can take a look, make comments, use it... whatever.
Speaking of
isActive()...
Another "accidental" overwritting... I will definitely abandone the "other" repository (the -typed one, grrrrrrr)
Regarding the display, it clearly says you should close it, but I have opted by leaving it open whilst the window is alive (I think this is the old ewmh approach, so it can not be that bad!). Your suggestion of opening it just once is a very good option but, what if a given setup has more than one display? My new implementation takes into account (not sure if properly), multi-display and multi-screen setups. Also may consider a window changing it root (I found nothing definite about it, but intuitively I think it could be possible).
Meanwhile, is it possible for you to release (or at least tag) commit 4b4a9ae ? This is the last commit after a bunch of improvements and bugfixes since 0.0.42 and right before this deeper rework and
ewmhreplacement.
I will very soon! I'm finishing the xlib container, and struggling with Linux' acceptInput() and sendBehind() methods (I think I finally got it! right last night... need to test). Once I finish both things (in a few days), I will upload a new version and will tag it!!!
In windows, GetMenu() stoped working after updating to Windows 11... I opend an issue at pywin32 site, but the author had the opinion that this is related to how Windows 11 creates menus, not python.
Another thing that I'm struggling with on windows is the alwaysOnTop() method. It works with apps using CGI calls, but not with those using DirectDraw exclusive mode... (one of my other reps is a SystemMonitor which should stay on top of EVERYTHIN, specifically games). I have some strings to pull, but they all need using C. If you are skilled in win32/C, I would really appreciate your help!
And, finally, I'm also struggling with the isAlerting property in windows (in macOS seems to be impossible).
These last three things are non-blocking for a new release.
Thank you SO MUCH for your help and feedback!!!
MestreLion
commented
1 year ago Do not worry at all! First things, first. To be honest, I thoght I had overwhelmed you because I realized I was so excited with the idea of the new module that I entered like an "elephant in a glass shop"!!! HAHAHA!
An elephant I really appreciate! That kind of feedback, ideas and suggestions are awesome, there's no such thing as "too much". I might not have the time to process them now, but they'll be invaluable once I resume the project.
My fault. I have two repositories to manage about typing (eternal hate) and very likely I overwrote the right version. ... Another "accidental" overwritting... I will definitely abandone the "other" repository (the -typed one, grrrrrrr)
Typing is a pain when you try to apply from scratch on a already mature codebase. But if done since the start it's quite cool, helped me catch many logical errors. It might be easier if you drop the separate .pyi stub files and fully annotate the codebase itself. Once you pass mypy --strict it's a joyride afterwards.
Speaking of overwriting, just found this, not sure if accident or intentional:
Default user value changed from True to False in 552f4797932fb55be4127f5ef54fb41466f366c7, which was the very issue that started this all
My fault again. What I sent you was a very preliminary version, still to be deeply reviewed and tested.
I'm talking about your repository, current master branch. It seems to be in a WIP state.
Regarding the display, it clearly says you should close it, but I have opted by leaving it open whilst the window is alive (I think this is the old ewmh approach, so it can not be that bad!)
Closing the display socket should never be your responsibility. And it prevents you from using a module-level singleton, as there is no way to know when a module "exits" or goes out of scope. That should be left to Python's garbage collector, or, if really needed, to Xlib's Display class itself.
I can create a PR to python-xlib to add a .__del__() method that calls its .close() to close the socket. It's their bug it's not there already.
Please share the quote (and source) about having to close the display. Is it from X, or C's XLib, or Python Xlib? Having an authoritative reference about this would greatly help such PR to get accepted by Python-Xlib
Meanwhile, is it possible for you to release (or at least tag) commit 4b4a9ae ? This is the last commit after a bunch of improvements and bugfixes since 0.0.42 and right before this deeper rework and
ewmhreplacement.I will very soon! I'm finishing the xlib container, and struggling with Linux' acceptInput() and sendBehind() methods (I think I finally got it! right last night... need to test). Once I finished both things (in a few days), I will upload a new version and will tag it!!!
That was exactly my point: massive changes like this take time, not only to develop but also to trim the rough edges afterwards. Meanwhile your master is in a WIP state, and 0.0.42 is missing important bugfixes (specially the py.typed inclusion). My suggestion is to create at least a simple 0.0.43 tag right now, so you can take your time to create the new release, be it in days or weeks or more.
In windows [...] ... If you are skilled in win32/C, I would really appreciate your help!
Not a Windows user for more than 10 years now, and I have never touched the win32 C API, or any C call from Python, sorry.
Kalmat
commented
1 year ago Typing is a pain when you try to apply from scratch on a already mature codebase. But if done since the start it's quite cool, helped me catch many logical errors. It might be easier if you drop the separate
.pyistub files and fully annotate the codebase itself. Once you passmypy --strictit's a joyride afterwards.
That must be only when you know what you're doing!!! I don't, unfortunately... it's a try-fail cycle... For instance, what is a .pyi file? I am checking the ewmh.pyi the other user builñt, but not sure if I understand. In case I build a .pyi, then I don't have to type within the module?
Speaking of overwriting, just found this, not sure if accident or intentional:
This time, semi-intentional. I was using this input param in other functions within xlibcontainer, I thought that using "1" was more natural (not to force, I mean), then I changed it in all functions... and forgot your change... sorry!!! We are still on time to revert it to True as default
I'm talking about your repository, current
masterbranch. It seems to be in a WIP state.
Absolutely it's WIP. I use github as backup in case something goes wrong with my PC... So I use to commit things that are "almost" done... is it a bad practice?
That was exactly my point: massive changes like this take time, not only to develop but also to trim the rough edges afterwards. Meanwhile your
masteris in a WIP state, and 0.0.42 is missing important bugfixes (specially thepy.typedinclusion). My suggestion is to create at least a simple 0.0.43 tag right now, so you can take your time to create the new release, be it in days or weeks or more.
You're right! I will do it tomorrow, I promise. Thank you for teaching me these things (I'm not a developer, I have never worked with VCS tools within a team, don't know the best practices and accepted uses... so, thank you!!)
Closing the display socket should never be your responsibility. And it prevents you from using a module-level singleton, as there is no way to know when a module "exits" or goes out of scope. That should be left to Python's garbage collector, or, if really needed, to Xlib's
Displayclass itself.
I have taken a totally new approach. Please (when you have time, of course, no hurries), take a look to this script (xlibcontainer.zip) I'm attaching it again to this reply (I have been workeing on it this evening and has some additions since my last reply). Besides, I think we have to think about the display issue very carefully. What if a given setup has more than one display? My new implementation takes into account (not sure if properly... how to f**g test it?!?!?!), multi-display and multi-screen setups. Also may consider (it's commented until I know for sure) a window changing its root (I found nothing definite about it, but intuitively I think it could be possible).
I found about Display issues picking here and there (about the possibility of a window changing its root, I found nothing ...yet). e.g. here: https://stackoverflow.com/questions/66158748/do-i-have-to-xfree-the-display-pointer-in-xlib
Besides, I read somewhere that there is a limited number of available displays connections (I think I remember somebody mentioning 256...).
Anyway, I think it's something to deeply re-think.
MestreLion
commented
1 year ago For instance, what is a .pyi file?
It's a python stub file for typing, you can also read more here
I am checking the ewmh.pyi the other user builñt, but not sure if I understand. In case I build a .pyi, then I don't have to type within the module?
Stub files are meant for cases when you cannot change the code itself, for example dependencies and 3rd-party modules. You see, when you use mypy --strict (the golden goal of typing), it requires not only your code to be typed, but also any other modules you import too! So how can you add type info on source code you don't own, such as ewmh or other dependencies? You create a stub file for it. For popular packages such as python-xlib, pillow, requests, someone else have done this already, and you can simply grab from the typeshed project (that's where the types-python-xlib extra dev dependency in your setup.py comes from)
If your dependency is not there, like ewmh, you have no choice but to create one yourself (or give up about mypy --strict - it's optional after all). And that user was kind enough to provide you an EWMH stub so you didn't have to :)
But that's for 3rd-party and dependencies. For your code it makes no sense to create a separate pyi file. You own the code, so it's much more straightforward to simply add typing annotations in the code itself. You don't need a stub, you have the real thing!
This time, semi-intentional. I was using this input param in other functions within xlibcontainer, I thought that using "1" was more natural (not to force, I mean)
Don't think of this as "force", remember this was how I called initially. The true intention is to be, as the name says, a source indicator: it tells apart an application in the background doing some stuff (for example, a torrent app notifying a download has finished and activating a file manager, or a chat app notifying you have a new message: both don't really care if the activation is ignored or not by the WM, they did their job); from a foreground action on behalf of the user, as if it was controlling and manipulating the windows him/herself.
In that sense, 2 is more natural for your library: it's called Py-Win-Ctl for a reason. You want to actually control windows. You do care if the WM ignores your requests. You're not sending an activation request to a file manager or chat window just as a convenience, and it's fine if it just flashes instead to notify the user. No, you want the window to respond to your actions. You are simulating user actions. That's what the 2 is actually about.
MestreLion
commented
1 year ago Absolutely it's WIP. I use github as backup in case something goes wrong with my PC... So I use to commit things that are "almost" done... is it a bad practice?
Well... I'd say it's unusual. Most projects use another branch for WIP stuff, like develop or testing, and keep master only for releases and stable commits. Some projects even forbid any developing on master, keeping it only for merging other branches when they're ready for production. That's overkill for small projects, sure, but it's always a good idea to keep master always stable and working, and use feature branches for WIP.
You're right! I will do it tomorrow, I promise. Thank you for teaching me these things (I'm not a developer, I have never worked with VCS tools within a team, don't know the best practices and accepted uses... so, thank you!!)
You're are not a developer? No sir, you are. A fine one, and PyWinCtl is a proof of that. Congrats :)
I have taken a totally new approach. Please (when you have time, of course, no hurries), take a look to this script (xlibcontainer.zip)
Will do, thanks!
Besides, I think we have to think about the display issue very carefully. What if a given setup has more than one display?
First, this is very, very uncommon. Most (if not all) linux distros have a single local display server. Even a second screen only happens in corner case scenarios such as VNC internal framebuffers for remote connections. Do not confuse an X display and screen with multi-monitor or even multi-GPU setups: even a 4 monitor setup is still typically connected to a single X display server at a single screen. So you could easily simply ignore multiple screens (let alone multiple displays) and just use the default for everything and no one will ever complain.
If you do want to support this, the EWMH approach was: create another ewmh.EWMH() instance. Each instance is bound to a (fixed) display+screen set at instantiation time. And that was a good design decision: you set it once, you connect only once, and all calls are methods of this instance, bound to the same display.
But ewmh could support this because EWMH is a Linux thing. PyWinCtl is cross-platform. And Windows and MacOS have no such concept of "X displays". How would you create an API for that? Would BaseWindow have Linux-only methods?
My new implementation takes into account (not sure if properly... how to f**g test it?!?!?!), multi-display and multi-screen setups.
For multi-displays you can SSH to a remote server (or a laptop in your LAN) with -X and run, say, Firefox or Gedit, so the window opens in your desktop but the app still runs on the laptop. From the point of view of your desktop, the laptop is another display.
For multiple screens, I have no idea. Maybe play with VNC and check if it runs as a virtual screen?
Do you really want to go down this rabbit hole?
Well, whatever you do, make sure you create a single Xlib.display.Display() instance per display+screen, and all calls such as getAllWindows() and getActiveWindow() use that same display object. Display selection should occur only one (you can expose a pywinctl.set_x11_display(...) if you want... it would then change the module-level singleton instance the other functions refer to)
Also may consider (it's commented until I know for sure) a window changing its root (I found nothing definite about it, but intuitively I think it could be possible).
I really don't think this is possible, it would mean changing its screen. (and that's not a monitor). Are you sure you're not confusing with changing its parent? Reparenting (within the same display+screen) is fine, and it does not affect PyWinCtl
I found about Display issues picking here and there (about the possibility of a window changing its root, I found nothing ...yet). e.g. here: https://stackoverflow.com/questions/66158748/do-i-have-to-xfree-the-display-pointer-in-xlib
That's for C XLib, it does not apply to Python. C has pointers and malloc and free(), so you have to destroy objects you've allocated, etc etc. python-xlib makes direct calls to the X server, it does not use the C library.
Besides, I read somewhere that there is a limited number of available displays connections (I think I remember somebody mentioning 256...).
Local servers perhaps? Not that you should care, any typical Desktop have a single X server running (and hence a single display)
Anyway, I think it's something to deeply re-think.
Or to stop over-thinking :-P
Just add this to the docs: _"On Linux, PyWinCtl methods work on the default X display, screen 0. To change this, call pywinctl.change_x11_display() (or set_x11_display(), or init_x11_display() etc) before using any other method`_
Kalmat
commented
1 year ago Wow! this is pure gold!
Well, as a summary:
Thank you!
MestreLion
commented
1 year ago I will open a new "dev" branch (I hope not to mess around with it. E.g. PyCharm asks for a "tracking branch"... should this one be master or dev?)
No idea. I use PyCharm too, but just as an Editor/IDE, not for git operations, those I prefer to do in a terminal. Creating a new dev branch where your current master is is as simple as
git branch dev And to tag commit 4b4a9aeb89ab40f436aea7b47dfd24275c3e044d, so you can refer to it later when creating the github and Pypi provisional 0.0.43 release:
git checkout 4b4a9aeb8
git tag 'v0.0.43'
git push --tags(and you're ready to build that version and upload to PyPi. Please do!)
Optionally, you could also revert master to that commit. Usually reverting branches is very very bad practice (specially master!), but depending on how WIP/funcional it is now, it might be a good idea to set it back to a stable commit:
git branch --force masterAnd then push everything to github:
git push --force --all --set-upstreamThis way your master will be at a stable commit (and released as 0.0.43), and your new dev branch will be at the tip of your WIP work. You can switch back and forth using git checkout dev / git checkout master, but most likely your master will stay still while you work on the revamp.
"Old" ewmh allowed to pass a display and a root.
(not root, but screen number. and not a "display", but merely its name)
It did, but it could afford to do so cos it had 2 luxuries PyWinCtl doesn't:
ewmh.EWMH() to hold the display and root instances as singletons. Functions like getAllWindows() were methods of this class. You don't have that: your getAllWindows() is a module-level function, so it must get the display/root instances from module-level globals. And you can't pass values to a module, so you must initialize them with a default value and at best provide a function to change them.So, it was somehow prepared for multi-display environments.
Humm, no, it wasn't. It was prepared for a different display, not multiple ones. If you wanted to connect to another display, you needed to create another top level ewmh.EWMH() instance object.
Not clear to me what could happen if the provided root didn't belong to the given display...
But we didn't provide a root to ewmh! We provided it a display name and a screen number, and ewmh would create a display connection and select a root based on those parameters. The root was bound to that display by definition.
Anyway, following your advice, I will stop "overthinking" about displays and roots. I will assume there is just one display in the system. In case any future user shows up with an issue, we will see...
That user could call pywinctl.change_x11_display(name: str, sno: int) and done, all future getAllWindows() calls would return windows from the chosen display / screen.
Anyway, I will still leave the functions that are able to gather the information on all displays/screens/roots, get the display/screen/root from a given window, etc... I guess these can be useful in some other situations.
They might, maybe, even if they don't translate well in a cross-platform API as windows and macOS have no equivalents. Also, you'll quickly find that both jobs (list local displays and get root from a window) are not trivial tasks.
Kalmat
commented
1 year ago I really love this kind of discussions... They are very interesting and very useful!
I have open a new "dev" branch... As expected, I messed with the checkouts and deleted all my work on local dev branch (fortunately AFTER committing to github). HAHAHAHA! Well, I think I fixed it and I have tagged a new "0.1" release. It's not ready to PyPi, but it will hopefuly be quite soon.
PS: To avoid the mess... Do I have to PR my own code??? PS2: I had a very bad time with typing... it's taking to me more time than "actual" features!!! PS·: PS1 and PS2 are definitely against my motto: "coding for fun"... grrrr (just kidding!!!)
Besides, you are right, old ewmh ignores root param:
def __init__(self, _display=None, root=None):
self.display = _display or display.Display()
self.root = root or self.display.screen().rootAnyway, the point is over. I will stick to default display. I was referring to leave getAllDisplaysInfo() and other related functions internally, I was not thinking to expose it as part of PyWinCtl. Sorry about the confusion. As you very sharply pointed out, PyWinCtl has its own rules. I hope xlibcontainer.py can be helpful for your future new-ewmh module. We can retake that exciting project when you have more time!!!
MestreLion
commented
1 year ago I have tagged a new "0.1" release. It's not ready to PyPi, but it will hopefuly be quite soon.
Tagging a 0.1 release based on WIP code is unnecessary. That tag can wait until all the WIP code finished
Meanwhile, what I (and the project) really need is a stable version on PyPi, based on commit 4b4a9aeb89ab40f436aea7b47dfd24275c3e044d
Quoting my earlier comments on this:
Meanwhile, is it possible for you to release (or at least tag) commit 4b4a9ae ? This is the last commit after a bunch of improvements and bugfixes since 0.0.42 and right before this deeper rework and
ewmhreplacement.massive changes like this take time, not only to develop but also to trim the rough edges afterwards. Meanwhile your
masteris in a WIP state, and 0.0.42 is missing important bugfixes (specially thepy.typedinclusion). My suggestion is to create at least a simple 0.0.43 tag right now, so you can take your time to create the new release, be it in days or weeks or more.And to tag commit 4b4a9ae, so you can refer to it later when creating the github and Pypi provisional 0.0.43 release:
git checkout 4b4a9aeb8 git tag 'v0.0.43' git push --tags(and you're ready to build that version and upload to PyPi. Please do!)
Please, please upload a provisional 0.0.43 version to PyPi. Commit 4b4a9aeb8 is stable, bug-free and ready for PyPi. I cant publish my own project into PyPI unless there's a PyWinCtl version there that matches the API my project is using.
All the other interesting points we're discussing here can wait until that.
Kalmat
commented
1 year ago Done!
Sorry because I was not aware of such a hurry. Version 0.1 was (hopefuly) not WIP. I tested it in all platforms and was working well. It's true that typing is stealing too much time from me, so I couldn't make a deep revision and tests. I was hoping to finish it today or tomorrow, so I was expecting to go forward and upload a full new 0.1 version everywhere. Anyway, everything is in the right place now, so I have more time to test. Besides, I didn't know (now I do) that github tags were so important!! I thought people uses PyPi, not github... Thank you!
Kalmat
commented
1 year ago oh, oh... after checkout, all new local content is gone?!?!?! Well, I hope I can recover it back... I didn't have time to review and upload to PyPi. I will do it this evening, promissed.
MestreLion
commented
1 year ago Well, in a sense PyPI releases are more important than Github, because it only accepts packages that depend on packages that are already in PyPI. So if in my setup.py I declare I depend on PyWinCtl > 0.0.42, such version must exist on PyPI or I cannot upload my project there (unlike a local pip install, where I can simply say pip install pywinctl @ git+https://github... and instruct it to install a specific repo/branch/commit
MestreLion
commented
1 year ago oh, oh... after checkout, all new local content is gone?!?!?! Well, I hope I can recover it back... I didn't have time to review and upload to PyPi. I will do it this evening, promissed.
Not gone, just on "detached mode" while you tag and upload to PyPI. You can always get back to a branch with git checkout master or git checkout dev
Kalmat
commented
1 year ago Done! Uploaded version 0.0.43 to PyPi.
I have to internalize this all, get accostumed to these processes and avoid messing around... Right now I'm not sure if everything is ok, to be honest. Let's hope it is!!!
MestreLion
commented
1 year ago Done! Uploaded version 0.0.43 to PyPi.
:+1: :100: :1st_place_medal:
I have to internalize this all, get accostumed to these processes and avoid messing around... Right now I'm not sure if everything is ok, to be honest. Let's hope it is!!!
Looks sweet now! PyPI is at a stable (yet modern) release, you have all the time you need to do the X11 revamp on dev, while master stays kind of frozen until dev is read to be merged.
PS: To avoid the mess... Do I have to PR my own code???
Well, sort of. When dev is ready to be integrated to master, you'll merge it locally, then push to github:
$ git checkout dev # as you probably already are right now
... # so some work, commits, etc
$ git checkout master # switch to master
$ git merge dev
$ git push --allA PR in github is nothing more than a web interface to merge branches from other remotes.
If you're new to git, I fully recommend this Git Pro book. It's very easy, short (almost like a tutorial), and sponsored by the git project itself. It was THE best reference I could have reference when I started using git. You'll love chapters 2 and specially 3
First of all, I've seen you already replaced the infamous
ewmhmodule for your own direct Xlib calls. Oh my, that must've been hard! What a huge effort, so congrats!Second, my apologies for being silent on my own
ewmhreplacement. I had to take another priority project, so I can't continue our amazing discussions and brainstorming for now. Still, all your ideas and comments are properly stored (thanks Github!) and ready to resume as soon as I have more spare time. It might take a while tho...That said, I've noticed a few things in your new architecture:
https://github.com/Kalmat/PyWinCtl/blob/1d568c0ce09620ccbb7f7ebaeb7769620b1b66a7/setup.py#L41
Any particular reason to change from
python-xlibback toxlib? As explained in here and mentioned in #41 , thexlibPypi package is a non official, unmaintained fork, its last release was more than 5 years ago!python-xlibis the one published by the official repository, is actively maintained, last release was less than 3 months ago,https://github.com/Kalmat/PyWinCtl/blob/1d568c0ce09620ccbb7f7ebaeb7769620b1b66a7/src/pywinctl/_pywinctl_linux.py#L97-L111
So... you're creating a new
Display()instance on every call? That's terribly inefficient! A display instance creates a new connection to an X11 server, and the way Xlib packages does that (find the local unix socket in the filesystem, cache all atoms, etc) makes this a very expensive call, should do preferably only once. The new approach has a huge performance penalty:Previously (in commit 61688a13f848dae61868919e773cc08ce2fce21e):
Current
master:From 73 micro seconds to 11 milli seconds! That's 150 times slower!
This would not be a big issue by itself, but considering this is
getActiveWIndow(), a function called virtually everywhere and every time (directly or via the omnipresent.isActiveproperty), this really hurts. Checking if a window is active should be (and it was) a very inexpensive call. 11ms kills any use in real-time applications such as my bot, where sending keypresses and taking screenshots are timed against the 17ms of a 60FPS frame.I believe the best approach in this case is to create a
displayinstance singleton and use it whenever needed. It could be a module-level singleton attribute or wrapped in a "controller" class (such as theEWMH()class in both the oldewmhand in my new project). Creating it as aWindowinstance attribute is not ideal, but not as a bad as doing so on every call. I've noticed you're already doing so in your_XWindowWrapperclass, so why isn'tLinuxWindow.isActiveusing it? Still, a newDisplay()instance per window should not be the goal.