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

[RAPPS-DB] Add generic wallpaper icon for wallpaper packs #293

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

whindsaks
Copy link
Contributor

The icon is a modified version of Tango preferences-desktop-wallpaper.

@whindsaks whindsaks added the enhancement New feature or request label Jan 22, 2025

[Generate]
Files = *
Dir = %SystemRoot%\Web\Wallpaper
Icon = ..\..\system32\desk.cpl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the need for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small hack to display an icon in Add/Remove programs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about %SystemRoot%\system32\desk.cpl then? It would look a bit less hacky.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that icon is relative to the install directory.

Rapps could perhaps support and use special handling for paths starting with % for the icon but it currently does not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was suggesting something like what's used in the Dir syntax (and basically, if whatever is given there as the value was passed to some PathCanonicalize function, maybe it would understand that after Expanding the environment strings in "install_dir%SystemRoot%\whatever", the thing would actually refer to the systemroot\whatever.
(I hope you understand what I mean here ^^)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without looking it up, I believe Rapps calculates the icon path as something like CStringW icopath = PathAppend(installdir, GetIniStr("Icon")); (pseudo code).

We could check if the first character is % and then don't prefix the install directory but the current code in master cannot handle this AFAIK.

@HBelusca
Copy link
Contributor

What are the sizes of the wallpaper.ico ?

@whindsaks
Copy link
Contributor Author

What are the sizes of the wallpaper.ico ?

32x32 and 48x48

@HBelusca
Copy link
Contributor

What are the sizes of the wallpaper.ico ?

32x32 and 48x48

Maybe add 16x16 too?

@whindsaks
Copy link
Contributor Author

Maybe add 16x16 too?

Rapps uses the normal Windows icon size for the icons in the app list, this will never be 16x16? I'll add it anyway.

@oleg-dubinskiy
Copy link
Contributor

oleg-dubinskiy commented Jan 22, 2025

Maybe add 16x16 too?

Rapps uses the normal Windows icon size for the icons in the app list, this will never be 16x16? I'll add it anyway.

They were 16x16 until 2020, when I've sent a #3144 PR with implementation of displaying icons for installed apps in Rapps. It was required because usually apps contain 32x32 icons those are defined in their registry entries ("Uninstall" section), and they were displayed ugly with 16x16 size. See reactos/reactos#3144 (comment).
16x16 might be useful for 0.4.14 for example still.

@julenuri julenuri merged commit 3376c8c into reactos:master Jan 22, 2025
1 check passed
@whindsaks whindsaks deleted the GenericWallIco branch January 22, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants