dannyedel / dspdfviewer

Dual-Screen PDF Viewer for latex-beamer
http://dspdfviewer.danny-edel.de
GNU General Public License v2.0
218 stars 27 forks source link

Embed system qt #120

Closed dannyedel closed 8 years ago

dannyedel commented 8 years ago

Fixes #119

This PR is complete when:

  1. Correct dynamic linking to the system Qt translations (for dynamic linking platforms)
  2. Enable static linking to the system Qt translations (if an appropriate CMake option is given)
dannyedel commented 8 years ago

@projekter, I tried but I can't get it to work with Qt5. (Seems to work with qt4)

it does copy the qt_de.qm file, but it is very small (200byte) and obviously does not contain the translations.

But, I also fixed the dynamic linking, using QLibraryInfo instead of hardcoded paths. Could you try to build this pullrequest without specifying the new cmake option, and check what happens on windows, when you

If it works by simply bundling the translation files into the setup.exe, this might be a lot easier (and more qt-standard) than trying to hardcode and copy the translation files into the application.

However if it doesn't work, I'll keep debugging and trying.

projekter commented 8 years ago

The qt_de.qm file is just a container that links to the real translations, namely

I compiled this PR with Qt5. Simply copying them in the program folder directly does not work. Before you made the translations statically linked, I put them in a subfolder "translations" in the program directory, which loaded all of them correctly. But this does no longer work, only the dspdfviewer translation is loaded, not Qt. But if I create a qt.conf which simply points to this translations folder (which should be standard, anyway), the translations are loaded correctly. So I can once again bundle the translations, but now together with the configuration file. It is just weird that [Paths]/Prefix points to the application directory and [Paths]/Translations=Translations, which is all equal to the defaults and should thus not be necessary.

dannyedel commented 8 years ago

Basically I swapped the code to the one recommended on the official website, asking QLibraryInfo for the path instead of relying on a compile-time-coded one.

I suppose that this approach would be better right now, since on Windows/MSVC you are giving the application its own folder inside C:/program files/dspdfviewer, so we have a place to put them; and all other platforms currently supported link to the system translations dynamically.

I still think I need to ship the application translation statically, since for example on Linux, I don't have a folder "for my own", dspdfviewer gets installed into /usr/bin along with all other programs on the system.

I'll drop the last patch from the pullrequest, and leave it at the correct-dynamic-linking state. Feel free to send a PR containing your qt.conf so it can get included.

(BTW: did you test what happens when you put an empty qt.conf into the folder? Since you only use all the defaults, maybe they only get activated if "any" qt.conf is there)

projekter commented 8 years ago

You are right, it's enough to have an empty qt.conf present.