Adjust Windows to match the Linux backend setup#503
Conversation
|
Thanks @halldorfannar 🥂 I'll take care of the documentation issues. |
|
There's some other issues that make me uneasy here. We'll fix them in a separate PR. |
|
|
||
| ### Package Restructure | ||
|
|
||
| `mss.windows` is now a **package** rather than a single module, with a `choose_impl()` dispatcher that mirrors the `mss.linux` architecture. This prepares the project for future Windows backends (such as a DXGI-based implementation) that will sit alongside the existing GDI code. |
There was a problem hiding this comment.
We don't need to mention the internal aspects: choose_impl, MSSImplWindows, BACKENDS, or mss.windows.gdi. Discussing internal implementation details in user-facing documentation risks confusion about what users are supposed to access.
It should be sufficient to say that the Windows stuff was restructured, the existing GDI backend is still the default - and currently only - implementation, and that this was done to prepare for a DXGI implementation.
jholveck
left a comment
jholveck
left a comment
There was a problem hiding this comment.
There's just a couple of cleanups I'd suggest.
We don't need compatibility regarding MSSImplWindows. Indeed, we certainly shouldn't advertise that, or other internal details, to users.
I'd also suggest naming the new backend "gdi" in the backend= parameter. The goal for "default" is to have MSS select the backend, but each backend is still explicitly selectable.
You might consider updating the docs that mention backends, where they currently say that only Linux uses this facility, but I'm not sure if discussing Windows at this point is going to be insightful to end users.
| from mss.base import MSSImplementation | ||
| from mss.exception import ScreenShotError | ||
|
|
||
| # Re-export the GDI implementation so existing code that references |
There was a problem hiding this comment.
Unnecessary; MSSImplWindows was never publicly released.
|
|
||
| __all__ = ["MSS"] | ||
|
|
||
| BACKENDS = ["default"] |
There was a problem hiding this comment.
I would suggest adding an explicit "gdi", similar to how the Linux implementation has all the backends available by explicit request. Currently, of course, this would be the same as "default". The intention, though, is that "default" will eventually be able to look at things like the Windows version to choose the backend to use.
| to system-managed memory that we can read from. | ||
|
|
||
| This has no Windows-specific constructor parameters. | ||
| This backend is selected by ``backend="default"`` and has no Windows-specific |
There was a problem hiding this comment.
As above, I suggest making "gdi" the name for this backend, and "default" essentially an alias.
3 participants
Changes proposed in this PR
The Windows implementation now has a backend dispatcher and one implementation,
gdi, which is the default.Added a new test to get 100% coverage on the
__init__.pymodule for this new Windows package. Also improved one type annotation, since I was already going over all this code../check.shpassed