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 Scaling Feature on Windows #87

Merged
merged 10 commits into from Jun 23, 2023
Merged

Conversation

beyluta
Copy link
Contributor

@beyluta beyluta commented Jan 31, 2023

  • Update the binary to the latest 2.0.0 version.
  • Add the --scale flag.

#55 Partial fix for this issue. They later requested the feature for both Windows & Linux
#5 Potential fix for this issue. I don't have multiple monitors, but the last comment mentioned it having to do with the 'tile' property #5 (comment)

@beyluta
Copy link
Contributor Author

beyluta commented Jan 31, 2023

My bad, I completely forgot to run the tests. I will take a look at them tomorrow and fix the issues.

@sindresorhus
Copy link
Owner

@beyluta https://github.com/sindresorhus/win-wallpaper/releases/tag/v3.0.0

Would you be able to also add support for ARM64? Could be a separate pull request.

@beyluta
Copy link
Contributor Author

beyluta commented Feb 17, 2023

@beyluta https://github.com/sindresorhus/win-wallpaper/releases/tag/v3.0.0

Would you be able to also add support for ARM64? Could be a separate pull request.

I'll take a look at it as soon as I can.

@beyluta
Copy link
Contributor Author

beyluta commented Feb 25, 2023

Note: I've been having some issues running the new binary on my device.

I am unsure why, but sometimes the backgrounds will fail to be set (they give the error 0x8007007A... but other times, the background does not set and I get no errors).

I googled the issue and it seems it might be something to do with my machine. Although I am unsure what exactly. I also got other error codes but I sadly did not save them and have been unable to reproduce ever since.

@sindresorhus
Copy link
Owner

I'll take a look at it as soon as I can.

Do you plan to do it in this pull request or a different one?

@sindresorhus
Copy link
Owner

I am unsure why, but sometimes the backgrounds will fail to be set (they give the error 0x8007007A... but other times, the background does not set and I get no errors).

It may be related to what this is fixing: sindresorhus/windows-wallpaper#11

@beyluta
Copy link
Contributor Author

beyluta commented Mar 11, 2023

I'll take a look at it as soon as I can.

Do you plan to do it in this pull request or a different one?

Different PR for ARM64 support.

@sindresorhus
Copy link
Owner

@beyluta
Copy link
Contributor Author

beyluta commented Mar 19, 2023

Can you update the binary: https://github.com/sindresorhus/win-wallpaper/releases/tag/v3.0.1

I am still having the same issue. I am using the same images as provided in the root of the directory. fixture.jpg and fixture2.jpg

Error: "Failed to set the desktop wallpaper: The data area passed to a system call is too small. (0x8007007A)"

Here is a screenshot of my cmd

image

Running the same command multiple times will eventually succeed. However this time the Wallpaper isn't being set at all.

@sindresorhus
Copy link
Owner

// @tangdaoyuan @a-isaiahharvey Any ideas?

@martial-plains
Copy link

Can you try running the program as administrator?

@beyluta
Copy link
Contributor Author

beyluta commented Mar 20, 2023

Can you try running the program as administrator?

Had no effect. Same symptoms.

Win 11 Version 22H2 (OS Build 22621.1413)

@martial-plains
Copy link

This might not be a good question, but did you check this box in the "General" tab in the "Properties" of the executable?
Screenshot 2023-03-21 100945

When I tried downloading the executable in a VM I made in Parallels (Win 11 Version 22H2, OS Build: 22621.1265) the file didn't seem like it was executing at all up until I opened Properties and saw that option. After I got the executable to do something by checking the box I encountered the error that appeared in your screenshot after a few times of setting the wallpaper between fixture.jpg and fixture-æøå.jpg. But after I checked "Run this program as an administrator" in the Properties of the executable under the Compatibility tab the ran the Windows Terminal as administrator (Because if I didn't it'd open the executable in another window and I wouldn't be able to see the output because the window would close after it's done executing) I noticed that there weren't any errors after trying it about 15 times consecutively.

I hope this helps solves your issue🙏And I'm not good at English so I'm sorry if my grammar is bad.

@beyluta
Copy link
Contributor Author

beyluta commented Mar 21, 2023

This might not be a good question, but did you check this box in the "General" tab in the "Properties" of the executable? Screenshot 2023-03-21 100945

When I tried downloading the executable in a VM I made in Parallels (Win 11 Version 22H2, OS Build: 22621.1265) the file didn't seem like it was executing at all up until I opened Properties and saw that option. After I got the executable to do something by checking the box I encountered the error that appeared in your screenshot after a few times of setting the wallpaper between fixture.jpg and fixture-æøå.jpg. But after I checked "Run this program as an administrator" in the Properties of the executable under the Compatibility tab the ran the Windows Terminal as administrator (Because if I didn't it'd open the executable in another window and I wouldn't be able to see the output because the window would close after it's done executing) I noticed that there weren't any errors after trying it about 15 times consecutively.

I hope this helps solves your issue🙏And I'm not good at English so I'm sorry if my grammar is bad.

Oh, you're right. I assumed that running vscode as an admin would also grant the same privileges to the terminal when running executables. It worked when I checked the "run as administrator" checkbox in the executable itself. The only downside is that it prompts me for privileges whenever executing the command.

But that aside it is now working as expected. It also passed the npm run test.

Is this behaviour acceptable? Or should it work without admin privileges?

@sindresorhus
Copy link
Owner

Would be nice to make it work for non-admin. But even if not, it should at least show a user-friendly error message instead of crashing.

@beyluta
Copy link
Contributor Author

beyluta commented Apr 5, 2023

Would be nice to make it work for non-admin. But even if not, it should at least show a user-friendly error message instead of crashing.

I'll see what I can do. Would the following error message suffice?
An error has occurred. Error code: XXX. Have you tried running the program as an administrator?

I can display something along these lines. The issue would still exist with the binary of course. Maybe someone is working to also display something similar?

@sindresorhus
Copy link
Owner

I meant that the binary should do this, not this package.

@martial-plains
Copy link

@beyluta Can you try running this binary?

https://github.com/a-isaiahharvey/WinWallpaper-CPP/releases/tag/1.0.0

For me I found it kind of strange that when I tried implementing nearly the exact same program in C++ I didn't get have any errors on my side nor had to run as administrator.

@beyluta
Copy link
Contributor Author

beyluta commented Apr 21, 2023

@beyluta Can you try running this binary?

https://github.com/a-isaiahharvey/WinWallpaper-CPP/releases/tag/1.0.0

For me I found it kind of strange that when I tried implementing nearly the exact same program in C++ I didn't get have any errors on my side nor had to run as administrator.

I tried running the binary but the set command wouldn't work for some reason. I used multiple pictures and formats but it just wouldn't set. I also tried giving the binary (and the cmd) admin privileges. See the image below (Same result using CMD or PowerShell).

I never got any error codes either.

image

@martial-plains
Copy link

Hmm. Try this update.
https://github.com/a-isaiahharvey/WinWallpaper-CPP/releases/tag/v1.1.0

I added more error handling and better support for relative file paths

@beyluta
Copy link
Contributor Author

beyluta commented Apr 21, 2023

Hmm. Try this update. https://github.com/a-isaiahharvey/WinWallpaper-CPP/releases/tag/v1.1.0

I added more error handling and better support for relative file paths

Whatever you did, it seems to have fixed the issue, now the backgrounds are being set. Also works without admin privileges. Scale feature also works nicely.

image

@martial-plains
Copy link

Whatever you did, it seems to have fixed the issue, now the backgrounds are being set. Also works without admin privileges. Scale feature also works nicely.

That's awesome! Now the only thing for me to do is figure out if it's possible to do the same thing in Rust without using admin privileges.

@sindresorhus
Copy link
Owner

Now the only thing for me to do is figure out if it's possible to do the same thing in Rust without using admin privileges.

@a-isaiahharvey Were you able to look into this?

@martial-plains
Copy link

@sindresorhus I tried to fix it from my end and it works. I would still need to verify that "The data area passed to a system call is too small." doesn't happen on another Windows computer to be sure though

@sindresorhus
Copy link
Owner

Could you compile a binary from sindresorhus/windows-wallpaper#13 that @beyluta can test?

@martial-plains
Copy link

Sure. Here you go:

wallpaper.zip

@beyluta
Copy link
Contributor Author

beyluta commented Jun 21, 2023

Sure. Here you go:

wallpaper.zip

Worked like a charm on my machine :)

I'll update the pr as soon as the new version of the binary is released.

@sindresorhus
Copy link
Owner

@sindresorhus sindresorhus merged commit a6408ab into sindresorhus:main Jun 23, 2023
0 of 3 checks passed
@sindresorhus
Copy link
Owner

Awesome. Thanks for working on this, both of you 👍

@sindresorhus
Copy link
Owner

@beyluta Are you up for doing the ARM64 PR now too? If not, I will do a new release right away.

@beyluta
Copy link
Contributor Author

beyluta commented Jun 23, 2023

@beyluta Are you up for doing the ARM64 PR now too? If not, I will do a new release right away.

You should probably do a new release for now. I'm busy as of recent. I will look into it when I get some more free time.

sunjw pushed a commit to sunjw/npm-wallpaper that referenced this pull request Mar 19, 2024
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.

None yet

3 participants