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

Improved code for supporting multiple Windows versions #93

Open
wants to merge 12 commits into
base: feature/windows-10-support
Choose a base branch
from

Conversation

Lej77
Copy link

@Lej77 Lej77 commented Apr 2, 2024

Hi again, I read your comment in my previous PR #92 and supporting multiple Windows versions definitively complicates the code so it makes sense to limit the scope of the project.

Anyway, I made some improvements to the code for supporting multiple Windows versions and I guess you can merge them into the same branch as the other Windows 10 support:

  • Added a multiple-windows-versions feature flag without which the code uses the previous approach of only supporting the latest COM interfaces.
    • Nearly no changes were made to the code outside of interfaces.rs (compared to the code in the rust branch).
    • Support for multiple Windows versions is done via interfaces_multi.rs which works as a drop in replacement for interfaces.rs when the feature flag is used.
  • Support same Windows versions as mzomparelli/VirtualDesktop: C# wrapper for the Virtual Desktop API on Windows 11..
  • Minimized work for adding new Windows versions.
    1. Add new module in interfaces_multi.rs
    2. In the new module declare the new COM interfaces.
    3. If the code in comobjects.rs or the other files uses new methods then add those to the definitions in build_dyn.rs.

DLL Size

I checked the size of the DLL (built in release mode) with and without the multiple-windows-versions feature:

  • Multiple Windows versions: 363 KB
  • Latest Windows version only: 342 KB

Tests

I also ran all the tests and they did pass on Windows 10 (well, except for the ones using features that weren't supported):

running 12 tests
test tests::test_desktop_count ... ok
test tests::test_desktop_get ... ok
test tests::test_kill_explorer_exe_manually ... ok
test tests::test_listener_manual ... ok
test tests::test_desktop_moves ... ok
test tests::test_errors ... ok
test tests::test_move_notepad_between_desktops ... ok
test tests::test_switch_desktops_rapidly_manual ... ok
test tests::test_pin_notepad ... ok
test tests::test_pin_notepad_app ... ok
test tests::test_rename_desktop ... FAILED
test tests::test_threads ... FAILED

@eythaann
Copy link

eythaann commented Aug 17, 2024

@Lej77 Hi, how do you get the IIDs and know what was changed for each windows version?

@Lej77
Copy link
Author

Lej77 commented Aug 17, 2024

@eythaann Unfortunately I didn't figure that out myself. I just used the same IIDs as Slion/VirtualDesktop: C# wrapper for the Virtual Desktop API on Windows 11.. I also looked at some earlier commtis in this repository. I link to some of these places in the docs for interfaces_multi.rs.

Under the Windows version support heading of the Slion/VirtualDesktop repo it is mentioned that you can use regedit to find the IIDs of the current Windows installation. So that could be worth trying but I don't know how to discover what functions exist on COM objects.

@eythaann
Copy link

eythaann commented Aug 17, 2024

oh I understand, I though that we can made the exposed api functions returns and error Err(UnsuportedWindowsVersion), actually the app just panics when is trying to get the unvalid COM object. I do a workaround just creating a helper function to check is I can use winvd, but return a Result::Err will be a easy way to handle error for all the Clients of this crate!.

edit: Also this won't be a breaking change, because the functions already return a Result enum.

image

@Lej77
Copy link
Author

Lej77 commented Aug 17, 2024

I guess you want to check if the hardcoded IID matches the actually IID for the COM interfaces and return your Err(UnsuportedWindowsVersion) if that isn't the case? That could help but we might not need to know the real IID to do this since if the IID is incorrect then the query_service call will likely fail (since the IID is unlikely to be used by another interface). One issue with this is that it still would not detect a version mismatch where the IID was the same but the interface definition changed.

This PR does have code to detect the current Windows version and that might be useful to return an error when a too old Windows version is used. We can't really return errors for too new versions since we don't know in advance when the interfaces will change.

@eythaann
Copy link

oh sure, I'm sure that windows-rs crate have a way to test if a IID is valid as a COM object before create it. so it could be the solution, In my case I'm using this crate for a module on my last project Seelen-UI and some users report crash on last 24h2. I for now I just will disable the Virtual desktop module for 24h2 and later I will try to do some changes in my fork to later be added to this PR, mostly to avoid cases where the program may crash without allowing the client to handle the error.

@Ciantic
Copy link
Owner

Ciantic commented Aug 26, 2024

FWIW, I know this doesn't sound helpful, but to me ideal multiple-version support is different DLLs.

On your application code look Windows version and LoadLibrary older DLL.

Windows 11 has already had 6 backward incompatible IIDs, and they are not just IIDs, they change the interfaces. Managing multiple old interfaces in one code base is quite difficult.

Ideally, I would keep only the latest code in the codebase and have old DLLs available.

@Lej77
Copy link
Author

Lej77 commented Aug 26, 2024

@Ciantic I can understand that perspective but I disagree that the tradeoff isn't worth it. This is your library of course so I will just continue to maintain a fork with my changes. There are many advantages to supporting multiple versions in the same library:

  • The binary size is smaller (363 KB vs about 10 * 300 KB = 3 000 KB).
  • It's easier to deploy a program that is statically linked than to bundle the executable with 10 or so dll files. (This is the largest issue I think.)
  • The old dll files might have bugs or missing features that are easier to update if we just release a single new dll file.
    • For example the Windows 10 dll was released before the COM interfaces were correctly defined and it also didn't include functionality for creating or removing virtual desktops. Both of which are fixed in my fork.
  • A program can still support newer versions by optionally loading a dll. I do this in my virtual-desktop-manager program with the vd.rs file.
  • I wanted an alternative to Slion/VirtualDesktop: C# wrapper for the Virtual Desktop API on Windows 11 and that library supports multiple versions.

The only disadvantage to supporting multiple versions is that it complicates the library code. That is quite a large issue but I think it is worth it, at least for me. It is quite self contained so it is really only the interfaces.rs file that needs changes, the rest of the library code can remain virtually unchanged.

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

Successfully merging this pull request may close these issues.

3 participants