Use Qt's Resource System (2nd approach) #1891

lukas-w · 2015-03-21T10:41:57Z

This PR does pretty much the same as #1742, but takes a less invasive approach, touching much less code. In contrast to my first approach, embed::getIconPixmap stays (I removed it and replaced all calls in #1742), but uses Qt's resource system and QDir's search paths internally.

~~Even though I didn't test it, it should now be~~ It's now possible to drop in SVGs instead of PNGs in themes without changing any code.

It's already been possible to replace any plugin's artwork by adding a file called something like e.g. monstro_logo.png to the theme folder. LMMS will then use that one instead of the plugin's built-in file. This PR changes this to monstro/logo.png instead (subdirectory vs underscore), just for the sake of easy implementation. I could not find any theme that replaces a plugin's artwork, so I hope this will have no impact on existing themes. Please let me know if I'm wrong and if there are such themes out there (on the LSP).

Support for generated .rc files was added in CMake v2.8.9 to the QT4_ADD_RESOURCES macro. In order to support older versions of CMake, this commit includes the source of the new macro from v2.8.9 and uses it if necessary.

Umcaruje · 2015-03-21T11:13:21Z

It's already been possible to replace any plugin's artwork by adding a file called something like e.g. monstro_logo.png to the theme folder. LMMS will then use that one instead of the plugin's built-in file. This PR changes this to monstro/logo.png instead (subdirectory vs underscore), just for the sake of easy implementation. I could not find any theme that replaces a plugin's artwork, so I hope this will have no impact on existing themes. Please let me know if I'm wrong and if there are such themes out there (on the LSP).

I think this won't be a problem, since theme compatibility is broken anyways (you can't use 1.1 themes on 1.2). This is a very useful feature and should be documented though.

So if I want to change the logo and/or the artwork of a plugin, I just add an png of the thing I want replaced? e.g. lb302/artwork.png and tripleoscillator/logo.png ?

softrabbit · 2015-03-21T11:27:02Z

Even though I didn't test it, it should still be possible to drop in SVGs instead of PNGs in themes without changing any code.

FTFY.

lukas-w · 2015-03-21T14:17:44Z

So if I want to change the logo and/or the artwork of a plugin, I just add an png of the thing I want replaced? e.g. lb302/artwork.png and tripleoscillator/logo.png ?

Exactly. ~~Note that I didn't test that one either.~~ Tested, works.

Even though I didn't test it, it should still be possible to drop in SVGs instead of PNGs in themes without changing any code.

FTFY.

Nope, that didn't work before. We didn't have any code capable of loading SVGs. It's now possible by using QImageReader which supports SVGs (see 9175928).

lukas-w · 2015-03-22T09:48:25Z

@softrabbit Nevertheless, I'd be interested to know what you're referring to. Maybe I'm just missing something. :)

softrabbit · 2015-03-22T14:57:47Z

This: #887 (OK, the explicit looping through possible file names was horrible, but I've loaded SVGs in LMMS for ages with that code...)

I'm guessing new QPixmap(filename) actually uses QImageReader behind the scenes. The documentation for QPixmap does hint at that in saying:

The complete list of supported file formats are available through the QImageReader::supportedImageFormats() and QImageWriter::supportedImageFormats() functions.

lukas-w · 2015-03-22T18:22:36Z

Ah thanks for the clarification. :) I didn't know QPixmap can read SVGs (maybe only on some systems?), it didn't work for me and isn't listed in the docs.

* Fix lmms maine executable resources

tresf · 2015-04-17T19:43:31Z

@lukas-w shall we shelve this for after the 1.2 release, or work to integrate it prior?

lukas-w · 2015-04-29T18:00:07Z

Seeing that 1.2 already has a lot of changes, it'd probably be wiser to merge this PR after 1.2.

jasp00 · 2017-01-04T10:53:41Z

Why not to remove the embedding altogether? Advantages:

Runtime will be simpler.
No recompilation because of artwork.
Architecture-dependent size will be smaller, which is good for distros.

lukas-w · 2017-01-06T16:28:48Z

Yeah, why not. When this PR is merged it's just a matter of removing the QRC logic and installing the resources via CMake instead. No need to touch any code IIRC.

tresf · 2017-01-06T16:36:43Z

Why not to remove the embedding altogether? Advantages:

For lmms.exe/lmms (binary)? Sure. For plugins? I'd argue this could break portability and strongly advocate against it.

jasp00 · 2017-01-06T16:52:35Z

For plugins? I'd argue this could break portability

What do you mean? An external image can be loaded portably.

tresf · 2017-01-06T16:57:25Z

An external image can be loaded portably.

It can, but it shouldn't. If a developer were to make a new LMMS plugin and distribute the binary, he should not need to write special instructions for users to extract images e.g. theme/foo/bar.

jasp00 · 2017-01-06T18:41:25Z

If a developer were to make a new LMMS plugin and distribute the binary, he should not need to write special instructions for users to extract images

Are users not used to run installers or install packages? Anyway, if both methods are kept, official plug-ins still benefit from:

No recompilation because of artwork.
Architecture-dependent size will be smaller, which is good for distros.

tresf · 2017-01-06T22:10:33Z

Are users not used to run installers or install packages?

Generally, no. Commercial plugins do this, but it is not common for community plugins and just adds unnecessary burden to the distributor. Good installers are a lot of work to get right cross-platform.

official plug-ins still benefit from: No recompilation because of artwork.

Sure, but if unofficial plugins eventually use our code for building, we should have embedding be the default behavior. #3204.

https://github.com/PaulBatchelor/lmms-reverbsc is a good use-case for our discussion.

Architecture-dependent size will be smaller, which is good for distros.

👍

jasp00 · 2017-01-07T00:55:42Z

it is not common for community plugins and just adds unnecessary burden to the distributor. Good installers are a lot of work to get right cross-platform.

Then let us use ZIP files for distribution, like the JAR, XPI, and OpenDocument cases. We can package Perl and Python plug-ins for LMMS, even multiple plug-ins sharing the artwork; just do not make the artwork to be part of the compilation. LMMS would be able to handle these ZIP files. The plug-in developer would run make plugin-package.

if unofficial plugins eventually use our code for building, we should have embedding be the default behavior.

Why that default? Third-party developers would also benefit from avoiding recompilation and providing separated packages, acting as a minidistro. Of course, if we give both options, it does not matter what the default is.

tresf · 2017-01-07T05:48:49Z

We can package Perl and Python plug-ins for LMMS, even multiple plug-ins sharing the artwork; just do not make the artwork to be part of the compilation.

When LMMS starts supporting non-compiled, scripted plugins, we can revisit this topic (assuming then the scripted plugins will adopt some common GUI framework). A gigantic part of our user base (> 1M downloads / year) is Windows and Windows doesn't yet ship with Perl or Python (Windows 10 may eventually change this).

Then let us use ZIP files for distribution, like the JAR, XPI, and OpenDocument cases.

Sure, but how about we start with project files first. :)

jasp00 · 2017-01-07T22:32:10Z

We would not remove the embedding method until a better alternative is developed.

Making a drastic change such as remove all embedded resources from all of our executables assumes there's no good in keeping it the way it is today.

What are the good points for embedding in official binaries? My suggestion is to keep both options, to embed or not to embed, which could be overridden per plug-in. If embedding is done, it should happen on install rather than on build.

tresf · 2017-01-07T23:24:00Z

What are the good points for embedding in official binaries?

None.

Umcaruje · 2017-03-26T11:47:45Z

@lukas-w now that we have diverged stable-1.2 from master, do you want to resolve the merge conflicts so we test and merge this?

lukas-w · 2017-03-26T11:52:39Z

do you want to resolve the merge conflicts so we test and merge this?

Already at it right now 😉

# Conflicts: # cmake/modules/BuildPlugin.cmake # plugins/Amplifier/logo.png # plugins/BassBooster/logo.png # plugins/Delay/logo.png # plugins/Eq/CMakeLists.txt # plugins/Eq/EqEffect.cpp # plugins/Eq/logo.png # plugins/Flanger/CMakeLists.txt # plugins/Flanger/logo.png # plugins/LadspaEffect/CMakeLists.txt # plugins/LadspaEffect/logo.png # plugins/SpectrumAnalyzer/logo.png # plugins/VstEffect/CMakeLists.txt # plugins/VstEffect/logo.png # plugins/carlabase/CMakeLists.txt # plugins/dynamics_processor/logo.png # plugins/ladspa_browser/logo.png # plugins/lb303/CMakeLists.txt # plugins/lb303/lb303.cpp # plugins/opl2/CMakeLists.txt # plugins/papu/CMakeLists.txt # plugins/peak_controller_effect/logo.png # plugins/stereo_enhancer/logo.png # plugins/stereo_matrix/logo.png # plugins/vestige/CMakeLists.txt # plugins/waveshaper/logo.png # plugins/zynaddsubfx/CMakeLists.txt # src/CMakeLists.txt # src/gui/GuiApplication.cpp # src/gui/embed.cpp

lukas-w · 2017-03-26T11:55:31Z

Done.

Umcaruje · 2017-03-26T17:28:05Z

So I tested this out, LMMS functions as expected, no visual artifacts of any sort.

I did find a bug with the plugin artwork replacement feature though. If the plugin has an underscore in its folder name, the artwork won't get replaced.
I had 4 folders for 4 plugins with a replacement artwork.png (or in monstro's case artwork_op.png and artwork_mat.png):

Only plugins without an underscore get their artwork replaced:

lukas-w · 2017-03-26T18:08:01Z

Thanks for testing.

If the plugin has an underscore in its folder name, the artwork won't get replaced.

That's just a mismatch between the directory names in plugins and the actual plugin name. You have to match the plugin name, so it's tripleoscillator instead of triple_oscillator.
You can find the correct name in lines similar to BUILD_PLUGIN(tripleoscillator …) in the respective CMakeLists.txt or in the resulting library file (libtripleoscillator.so or tripleoscillator.dll).

So I guess this is good to merge?

Umcaruje · 2017-03-26T18:42:07Z

That's just a mismatch between the directory names in plugins and the actual plugin name. You have to match the plugin name, so it's tripleoscillator instead of triple_oscillator.

Thanks, that's good to know.

So I guess this is good to merge?

LGTM 👍 I can't comment on the code though, not quite versed in cmake.

lukas-w · 2017-03-26T20:06:01Z

I can't comment on the code though

It's been two years and no one's complained about the code :) Merging

* Remove bin2res, use Qt's resource system * Use QDir search paths and QImageReader in getIconPixmap * Don't include "embed.cpp" in plugins * getIconPixmap: Use QPixmapCache, use QPixmap::fromImageReader * Require CMake 2.8.9 * Fix ReverbSC embed usage

lukas-w added 5 commits March 21, 2015 09:50

Remove bin2res, use Qt's resource system

6591974

Fix Qt Resources for CMake < v2.8.9

38d825f

Support for generated .rc files was added in CMake v2.8.9 to the QT4_ADD_RESOURCES macro. In order to support older versions of CMake, this commit includes the source of the new macro from v2.8.9 and uses it if necessary.

Plugins: Remove meaningless standard logos

5e0d218

Use QDir search paths and QImageReader in getIconPixmap

9175928

Don't include "embed.cpp" in plugins

a457716

lukas-w mentioned this pull request Mar 21, 2015

Use Qt's Resource System #1742

Closed

getIconPixmap: Use QPixmapCache, use QPixmap::fromImageReader

ecfc404

* Fix lmms maine executable resources

lukas-w mentioned this pull request Aug 6, 2015

Cleanup GitHub Root Dir #2201

Closed

tresf force-pushed the master branch 2 times, most recently from 8c45c1f to 4da73f3 Compare September 15, 2015 18:32

Umcaruje mentioned this pull request May 18, 2016

Widgets left not themeable #2770

Open

7 tasks

tresf added this to the 1.3.0 milestone Jul 5, 2016

tresf mentioned this pull request Jan 4, 2017

Do not use LOCATION, CMP0026 #3203

Closed

lukas-w added 2 commits March 26, 2017 14:01

Require CMake 2.8.9

cc56dd4

Fix ReverbSC embed usage

01e068d

lukas-w merged commit 9f905bc into master Mar 26, 2017

lukas-w deleted the qrc2 branch March 26, 2017 20:06

tresf mentioned this pull request Apr 11, 2017

VeSTige preset left and right arrows are gone #3498

Closed

PhysSong mentioned this pull request Jul 6, 2017

Merge stable-1.2 into master #3607

Closed

karmux mentioned this pull request Jul 15, 2017

Support VST windows on Qt5 #3497

Closed

lukas-w mentioned this pull request Nov 22, 2017

CMake warning #3985

Closed

PhysSong mentioned this pull request Nov 22, 2017

Fix CMake warning #4002

Merged

PhysSong mentioned this pull request Jul 30, 2018

Running Cmake warns you about Policy CMP0072 not being set #4499

Open

winniehell mentioned this pull request Aug 11, 2019

Split CSS file into smaller ones #5120

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Qt's Resource System (2nd approach) #1891

Use Qt's Resource System (2nd approach) #1891

lukas-w commented Mar 21, 2015

Umcaruje commented Mar 21, 2015

softrabbit commented Mar 21, 2015

lukas-w commented Mar 21, 2015

lukas-w commented Mar 22, 2015

softrabbit commented Mar 22, 2015

lukas-w commented Mar 22, 2015

tresf commented Apr 17, 2015

lukas-w commented Apr 29, 2015

jasp00 commented Jan 4, 2017

lukas-w commented Jan 6, 2017

tresf commented Jan 6, 2017 •

edited

Loading

jasp00 commented Jan 6, 2017

tresf commented Jan 6, 2017 •

edited

Loading

jasp00 commented Jan 6, 2017

tresf commented Jan 6, 2017

jasp00 commented Jan 7, 2017

tresf commented Jan 7, 2017

jasp00 commented Jan 7, 2017

tresf commented Jan 7, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

lukas-w commented Mar 26, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

Use Qt's Resource System (2nd approach) #1891

Use Qt's Resource System (2nd approach) #1891

Conversation

lukas-w commented Mar 21, 2015

Umcaruje commented Mar 21, 2015

softrabbit commented Mar 21, 2015

lukas-w commented Mar 21, 2015

lukas-w commented Mar 22, 2015

softrabbit commented Mar 22, 2015

lukas-w commented Mar 22, 2015

tresf commented Apr 17, 2015

lukas-w commented Apr 29, 2015

jasp00 commented Jan 4, 2017

lukas-w commented Jan 6, 2017

tresf commented Jan 6, 2017 • edited Loading

jasp00 commented Jan 6, 2017

tresf commented Jan 6, 2017 • edited Loading

jasp00 commented Jan 6, 2017

tresf commented Jan 6, 2017

jasp00 commented Jan 7, 2017

tresf commented Jan 7, 2017

jasp00 commented Jan 7, 2017

tresf commented Jan 7, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

lukas-w commented Mar 26, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

Umcaruje commented Mar 26, 2017

lukas-w commented Mar 26, 2017

tresf commented Jan 6, 2017 •

edited

Loading

tresf commented Jan 6, 2017 •

edited

Loading