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

Update Windows document icon #1082

Closed
kintel opened this issue Dec 17, 2014 · 10 comments
Closed

Update Windows document icon #1082

kintel opened this issue Dec 17, 2014 · 10 comments

Comments

@kintel
Copy link
Member

@kintel kintel commented Dec 17, 2014

On Windows, the document icon is the same as the application icon. This is confusing.
Look at e.g. the document icon we use on Mac: https://github.com/openscad/openscad/blob/master/icons/SCAD.png

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 8, 2015

The mechanism for this appears to be documented at:
http://msdn.microsoft.com/en-us/library/windows/desktop/ms682212%28v=vs.85%29.aspx

It's a registry key entry: Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Classes\OpenSCAD_File\DefaultIcon
and can refer to an icon in the app as a resource or as a .ico.

Given that the distribution doesn't include any ico files, the registry method is appealing. However, I don't see the SCAD.png file in the openscad.qrc resource file.

I'll set up a branch, add the SCAD.png to resources and see if I can force the doc icon to it.

@kintel
Copy link
Member Author

@kintel kintel commented Jan 8, 2015

Note that the SCAD.png is based on the standard document icon for Mac OS X. W might want to redraw them to look like native document icons for Windows. But adding support for it is a good start.

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 8, 2015

First pass suggests a QSettings approach:

QSettings hklm("HKEY_LOCAL_MACHINE\SOFTWARE\Classes\OpenSCAD_File",QSettings::NativeFormat);
// Then edit subkeys.
hklm.setValue("DefaultIcon", "\"C:\\Program Files (x86)\OpenSCAD\openscad.exe,-10\"");

Or somesuch depending on the resource ID. I'll also have to come up to speed on how/when in the install the .exe location is determined and where the OS dependent code is managed.

I see decent Qt example code around the web dealing with windows registries. (http://phonon-vlc-mplayer.googlecode.com/svn/trunk/quarkplayer/WinDefaultApplication.cpp)

I've built OpenSCAD on windows many dozens of times by now, but I've haven't built the installer (which I'm imagining could be an appealing place for this.) So that's also on my list.

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 8, 2015

I'm new to nullsoft, but it kinda looks like this might end up being a one-liner in /scripts/installer.nsi

fingers crossed.

Of course, if everyone is getting excited about a preferences pile-on, the doc icon could be editable there as well ;-)

This is a super fun project. I'm new to Qt, C++, MSYS2, OpenGL, NSIS and Git so there's lots of interesting things to learn. (I have worked in wxWidgets/Tkinter, C/C#, Cygwin, DirectX, Windows Installer and SVN, so it's mostly a through the looking glass translation thing :-)

@MichaelAtOz
Copy link
Member

@MichaelAtOz MichaelAtOz commented Jan 8, 2015

Note, I'm not into the detail, but the mention of installer, I remind everyone that many use the zip, not the installer.

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 8, 2015

Note, I'm not into the detail, but the mention of installer, I remind everyone that many use the zip, not the installer.

Excellent point. That suggests some check on startup to guarantee the icon or something in preferences to allow it to change if the user wants something different.

To go after the first (which I think is the desired behavior,) I'll look at how/where things like file associations are set up to see what that pattern is like.

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 10, 2015

Update.

This took a surprising amount of digging. Ultimately, what has to happen is that the registry key:
HKEY_CLASSES_ROOT\OpenSCAD_File\has to have a new key created:
HKEY_CLASSES_ROOT\OpenSCAD_File\DefaultIcon which will require a default value of type REG_EXPAND_SZ that points to an icon.

Since OpenSCAD doesn't distribute with additional image files, this means a new icon needs to be built into the .exe. This is accomplished by adding a line to the end of openscad_win32.rc file consisting of something like the following:
IDI_ICON2 ICON DISCARDABLE "icons/openscad_doc.ico"
which will get built into the exe as icon index 1 (icon index 0 is the current app icon.)

This is referencing a new icon file in the icons subdir which I created with an icon editor:
image
The icon file contains different size icon images, so that Windows can pick the best for the display shown. IMHO, we should really do this for the main app icon as well.

You can see the large version below:
image

Setting this registry key outside the installer (since, as noted above, many folks use the zip file,) will require doing so when the app has launched.

I imagine the best place to invoke this is in the openscad.cc/main method, calling a new PlatformUtils_Win::setDefaultIcon method. This method would consist of something like:

    QSettings settings(HKEY_CLASSES_ROOT, QSettings::NativeFormat);
    settings.setValue(QLatin1String("OpenSCAD_File/DefaultIcon/Default"),appLocation + ",1");

where appLocation is the location of the openscad.exe (Which I still have to figure out how to find.) (This is still essentially pseudo-code at this point.)

So, I'm closer, but still have a ways to go.

@t-paul
Copy link
Member

@t-paul t-paul commented Jan 11, 2015

Nice! The path to the exe file is available via std::string PlatformUtils::applicationPath().

@TDeagan
Copy link
Contributor

@TDeagan TDeagan commented Jan 12, 2015

Whew, after a long slog through std::string and QVariant conversion then a lengthy hike through the intricacies of HKEY_CLASSES_ROOT vs. HKEY_CURRENT_USER, I've got working code.

In the cmdline and gui functions in openscad.cc I added:

#ifdef Q_OS_WIN
    QSettings reg_setting(QLatin1String("HKEY_CURRENT_USER"), QSettings::NativeFormat);
    QString appPath = QDir::toNativeSeparators(app.applicationFilePath() + QLatin1String(",1"));
    reg_setting.setValue(QLatin1String("Software/Classes/OpenSCAD_File/DefaultIcon/Default"),QVariant(appPath));
#endif

I tried moving this into PlatformUtils_win.cc and as a separate function in openscad.cc, but I kept running into problems with complaints about private nature of the app object and the way it's a QApplication object in gui and a QCoreApplication object in cmdline and such. Ultimately, there was a lot less surgery all around to just dupe the block above in gui and cmdline. I'm happy to keep working on that if it provides value in some way.

Notes:

  • Changing this registry key only takes effect when:
    • Computer restarts
    • User logs off and logs on again
    • explorer.exe is restarted (which is the relevant part of what's going on in the first two)
  • Setting the key in HKEY_CURRENT_USER works, but only for the current user. Not for 'all users'. That would require setting in HKEY_LOCAL_MACHINE, and that would take running in admin mode. Otherwise, it throws a QSettings::AccessError (doesn't break the app, the error is only discernible with a QSettings.status() call.) Even in admin mode the app pops up a warning modal asking the user if they want to allow changes to the system. Of course, any user who launches OpenSCAD will reset it for themselves.

So, if that works for everyone, I'm ready to do a pull request. But I'm happy to keep working on this if changes are desired. I've gone back and forth about only writing the registry key if it's not already populated with the desired value. It's a tiny bit more elegant, but there are some arguments for the bulletproof nature of just setting it every launch.

Currently, the only two files with changes are:

  • openscad_win32.rc
  • openscad.cc

Comments?

@kintel
Copy link
Member Author

@kintel kintel commented Jan 16, 2015

Fixed by #1160

@kintel kintel closed this Jan 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.