Re: We now include both png and svgz?

[Georg Baum]

Scott Kostyshak wrote:

> From what I understand, there is support for the change. I think Georg's
> main concern was that on some systems it might not work, but Enrico
> seems confident that it will work even on less commonly used systems and
> even for 4.8.x.

This is fine with me as well, I had no concern, I only wanted to point to 
the contradiction.


Georg

Re: We now include both png and svgz?

[Scott Kostyshak]

On Mon, Dec 07, 2015 at 08:43:03PM +0100, Georg Baum wrote:
> Scott Kostyshak wrote:
> 
> > From what I understand, there is support for the change. I think Georg's
> > main concern was that on some systems it might not work, but Enrico
> > seems confident that it will work even on less commonly used systems and
> > even for 4.8.x.
> 
> This is fine with me as well, I had no concern, I only wanted to point to 
> the contradiction.

Yes I'm glad you did. I think it is the right thing to do and also is
something that is best done in a major release. Hopefully there will not
be an issue.

Scott


signature.asc
Description: PGP signature

compilation error with Re: We now include both png and svgz?

[Uwe Stöhr]

Am 07.12.2015 um 19:48 schrieb Enrico Forestieri:


Done at db363ab1.


Now I get a compilation error:

  Generating qrc_Resources.cpp
  RCC: Error in 
'D:/LyXGit/Master/compile-2013/src/frontends/qt4/Resources.qrc' : Cannot 
find file 'D:/LyXGit/Master/lib/images/all-changes-accept.png'


maybe you removed to much?

thanks and regards
uwe

Re: compilation error with Re: We now include both png and svgz?

[Kornel Benko]

Am Montag, 7. Dezember 2015 um 22:10:35, schrieb Uwe Stöhr 
> Am 07.12.2015 um 19:48 schrieb Enrico Forestieri:
> 
> > Done at db363ab1.
> 
> Now I get a compilation error:
> 
>Generating qrc_Resources.cpp
>RCC: Error in 
> 'D:/LyXGit/Master/compile-2013/src/frontends/qt4/Resources.qrc' : Cannot 
> find file 'D:/LyXGit/Master/lib/images/all-changes-accept.png'

I know this one. It is the old Resources.qrc having still the old references, 
you have to remove it first.
It is only rebuild if it does not exist.

Dependencies don't help here unfortunately.

> maybe you removed to much?
> 
> thanks and regards
> uwe

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: compilation error with Re: We now include both png and svgz?

[Kornel Benko]

Am Montag, 7. Dezember 2015 um 22:38:48, schrieb Kornel Benko 
> Am Montag, 7. Dezember 2015 um 22:10:35, schrieb Uwe Stöhr 
> > Am 07.12.2015 um 19:48 schrieb Enrico Forestieri:
> > 
> > > Done at db363ab1.
> > 
> > Now I get a compilation error:
> > 
> >Generating qrc_Resources.cpp
> >RCC: Error in 
> > 'D:/LyXGit/Master/compile-2013/src/frontends/qt4/Resources.qrc' : Cannot 
> > find file 'D:/LyXGit/Master/lib/images/all-changes-accept.png'
> 
> I know this one. It is the old Resources.qrc having still the old references, 
> you have to remove it first.
> It is only rebuild if it does not exist.
> 
> Dependencies don't help here unfortunately.

That is not enough, you have also remove the object files.
The complete list (in the build-dir) is (linux, but windows should be simmilar)

src/frontends/qt4/Resources.qrc
src/frontends/qt4/qrc_Resources.cpp
src/frontends/qt4/CMakeFiles/frontend_qt.dir/qrc_Resources.cpp.o

and than reconfigure.

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: compilation error with Re: We now include both png and svgz?

[Uwe Stöhr]

Am 07.12.2015 um 22:53 schrieb Kornel Benko:


That is not enough, you have also remove the object files



many thanks! Now the error is gone.

best regards
Uwe

Re: We now include both png and svgz?

[Enrico Forestieri]

On Sun, Dec 06, 2015 at 09:12:11PM -0500, Scott Kostyshak wrote:
> On Sun, Dec 06, 2015 at 03:08:47PM +0100, Enrico Forestieri wrote:
> > On Sun, Dec 06, 2015 at 11:52:14AM +0100, Georg Baum wrote:
> > 
> > > Jean-Marc Lasgouttes wrote:
> > > 
> > > > Le 06/12/15 00:02, Scott Kostyshak a écrit :
> > > >> There was an issue on Windows that came from not including a .dll. Now
> > > >> that the issue has been fixed, we should decide if we would like to
> > > >> consider removing the .pngs. If we would like to remove the .pngs for
> > > >> 2.2.0 I think we should definitely do this for beta1. Conversely, if we
> > > >> remove the .pngs for beta, I think it would still be OK to add them 
> > > >> back
> > > >> for the final release if we discover problems, as JMarc proposed [1].
> > > > 
> > > > I think we should remove them.
> > > 
> > > Me too. As I wrote in the bug report, the current state does not make 
> > > sense. 
> > > If we do not remove the .pngs for some reason, then we need to change the 
> > > search algorithm to use .png them if loading .svg fails.
> > 
> > The attached script and patch get rid of the unneeded pngs.
> 
> From what I understand, there is support for the change. I think Georg's
> main concern was that on some systems it might not work, but Enrico
> seems confident that it will work even on less commonly used systems and
> even for 4.8.x.
> 
> Enrico, I would say commit. Thanks for the patch.

Done at db363ab1.

-- 
Enrico

Re: We now include both png and svgz?

[Enrico Forestieri]

On Sun, Dec 06, 2015 at 11:52:14AM +0100, Georg Baum wrote:

> Jean-Marc Lasgouttes wrote:
> 
> > Le 06/12/15 00:02, Scott Kostyshak a écrit :
> >> There was an issue on Windows that came from not including a .dll. Now
> >> that the issue has been fixed, we should decide if we would like to
> >> consider removing the .pngs. If we would like to remove the .pngs for
> >> 2.2.0 I think we should definitely do this for beta1. Conversely, if we
> >> remove the .pngs for beta, I think it would still be OK to add them back
> >> for the final release if we discover problems, as JMarc proposed [1].
> > 
> > I think we should remove them.
> 
> Me too. As I wrote in the bug report, the current state does not make sense. 
> If we do not remove the .pngs for some reason, then we need to change the 
> search algorithm to use .png them if loading .svg fails.

The attached script and patch get rid of the unneeded pngs.

-- 
Enrico


rmpng.sh
Description: Bourne shell script
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 63ebfba..62d10d4 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -19,7 +19,7 @@ dist_noinst_DATA = \
fonts/stmary10.sfd \
fonts/test/stmary10.lyx \
images/README \
-   images/font-smallcaps.png \
+   images/font-smallcaps.svgz \
images/math/ams_arrows.png \
images/math/ams_misc.png \
images/math/ams_nrel.png \
@@ -28,13 +28,12 @@ dist_noinst_DATA = \
images/math/arrows.png \
images/math/bop.png \
images/math/brel.png \
-   images/math/deco.png \
-   images/math/deco.png \
-   images/math/delim0.png \
-   images/math/delim1.png \
-   images/math/delim.png \
-   images/math/dots.png \
-   images/math/font.png \
+   images/math/deco.svgz \
+   images/math/delim0.svgz \
+   images/math/delim1.svgz \
+   images/math/delim.svgz \
+   images/math/dots.svgz \
+   images/math/font.svgz \
images/math/greek.png \
images/math/misc.png \
images/math/varsz.png
@@ -373,196 +372,193 @@ dist_fonts_DATA = \
 
 imagesdir = $(pkgdatadir)/images
 dist_images_DATA1X = \
-   images/all-changes-accept.png \
-   images/all-changes-reject.png \
-   images/banner.png \
-   images/bookmark-goto.png \
-   images/bookmark-goto_0.png \
-   images/bookmark-save.png \
-   images/box-insert.png \
-   images/break-line.png \
-   images/buffer-close.png \
-   images/buffer-export.png \
-   images/buffer-export_dvi.png \
-   images/buffer-export_dvi3.png \
-   images/buffer-export_latex.png \
-   images/buffer-export_pdf.png \
-   images/buffer-export_pdf2.png \
-   images/buffer-export_pdf3.png \
-   images/buffer-export_pdf4.png \
-   images/buffer-export_pdf5.png \
-   images/buffer-export_ps.png \
-   images/buffer-export_text.png \
-   images/buffer-new.png \
-   images/buffer-reload.png \
-   images/buffer-toggle-output-sync.png \
-   images/buffer-update.png \
-   images/buffer-update_dvi.png \
-   images/buffer-update_dvi3.png \
-   images/buffer-update_pdf.png \
-   images/buffer-update_pdf2.png \
-   images/buffer-update_pdf3.png \
-   images/buffer-update_pdf4.png \
-   images/buffer-update_pdf5.png \
-   images/buffer-update_ps.png \
-   images/buffer-view.png \
-   images/buffer-view_dvi.png \
-   images/buffer-view_dvi3.png \
-   images/buffer-view_pdf.png \
-   images/buffer-view_pdf2.png \
-   images/buffer-view_pdf3.png \
-   images/buffer-view_pdf4.png \
-   images/buffer-view_pdf5.png \
-   images/buffer-view_ps.png \
-   images/buffer-write-as.png \
-   images/buffer-write.png \
-   images/build-program.png \
+   images/all-changes-accept.svgz \
+   images/all-changes-reject.svgz \
+   images/banner.svgz \
+   images/bookmark-goto.svgz \
+   images/bookmark-goto_0.svgz \
+   images/bookmark-save.svgz \
+   images/box-insert.svgz \
+   images/break-line.svgz \
+   images/buffer-close.svgz \
+   images/buffer-export.svgz \
+   images/buffer-export_dvi.svgz \
+   images/buffer-export_dvi3.svgz \
+   images/buffer-export_latex.svgz \
+   images/buffer-export_pdf.svgz \
+   images/buffer-export_pdf2.svgz \
+   images/buffer-export_pdf3.svgz \
+   images/buffer-export_pdf4.svgz \
+   images/buffer-export_pdf5.svgz \
+   images/buffer-export_ps.svgz \
+   images/buffer-export_text.svgz \
+   images/buffer-new.svgz \
+   images/buffer-reload.svgz \
+   images/buffer-toggle-output-sync.svgz \
+   images/buffer-update.svgz \
+   images/buffer-update_dvi.svgz \
+   images/buffer-update_dvi3.svgz \
+   images/buffer-update_pdf.svgz \
+   images/buffer-update_pdf2.svgz \
+   images/buffer-update_pdf3.svgz \
+   images/buffer-update_pdf4.svgz \
+   

Re: We now include both png and svgz?

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Le 06/12/15 00:02, Scott Kostyshak a écrit :
>> There was an issue on Windows that came from not including a .dll. Now
>> that the issue has been fixed, we should decide if we would like to
>> consider removing the .pngs. If we would like to remove the .pngs for
>> 2.2.0 I think we should definitely do this for beta1. Conversely, if we
>> remove the .pngs for beta, I think it would still be OK to add them back
>> for the final release if we discover problems, as JMarc proposed [1].
> 
> I think we should remove them.

Me too. As I wrote in the bug report, the current state does not make sense. 
If we do not remove the .pngs for some reason, then we need to change the 
search algorithm to use .png them if loading .svg fails.


Georg

Re: We now include both png and svgz?

[Scott Kostyshak]

On Sun, Dec 06, 2015 at 03:08:47PM +0100, Enrico Forestieri wrote:
> On Sun, Dec 06, 2015 at 11:52:14AM +0100, Georg Baum wrote:
> 
> > Jean-Marc Lasgouttes wrote:
> > 
> > > Le 06/12/15 00:02, Scott Kostyshak a écrit :
> > >> There was an issue on Windows that came from not including a .dll. Now
> > >> that the issue has been fixed, we should decide if we would like to
> > >> consider removing the .pngs. If we would like to remove the .pngs for
> > >> 2.2.0 I think we should definitely do this for beta1. Conversely, if we
> > >> remove the .pngs for beta, I think it would still be OK to add them back
> > >> for the final release if we discover problems, as JMarc proposed [1].
> > > 
> > > I think we should remove them.
> > 
> > Me too. As I wrote in the bug report, the current state does not make 
> > sense. 
> > If we do not remove the .pngs for some reason, then we need to change the 
> > search algorithm to use .png them if loading .svg fails.
> 
> The attached script and patch get rid of the unneeded pngs.

From what I understand, there is support for the change. I think Georg's
main concern was that on some systems it might not work, but Enrico
seems confident that it will work even on less commonly used systems and
even for 4.8.x.

Enrico, I would say commit. Thanks for the patch.

Scott


signature.asc
Description: PGP signature

Re: We now include both png and svgz?

[Jean-Marc Lasgouttes]

Le 06/12/15 00:02, Scott Kostyshak a écrit :

There was an issue on Windows that came from not including a .dll. Now
that the issue has been fixed, we should decide if we would like to
consider removing the .pngs. If we would like to remove the .pngs for
2.2.0 I think we should definitely do this for beta1. Conversely, if we
remove the .pngs for beta, I think it would still be OK to add them back
for the final release if we discover problems, as JMarc proposed [1].


I think we should remove them.

JMarc

Re: We now include both png and svgz?

[Scott Kostyshak]

On Mon, Nov 16, 2015 at 06:01:57PM +0100, Enrico Forestieri wrote:
> On Sun, Nov 15, 2015 at 05:57:00PM +0100, Georg Baum wrote:
> 
> > Enrico Forestieri wrote:
> > 
> > > It is not intended to work that way. If the svgz image is there, it is
> > > not expected that Qt is not able to show it. The double entry are there
> > > only to assure that if there is no svgz then the png can be used instead.
> > > In this way, you are able to override the default icons by placing your
> > > own in the ~/.lyx/images directory, even if they are pngs.
> > 
> > OK, if it is assumed that qt can show svg images, then it does not make 
> > sense to ship or own .pngs. If we want to stick with this, we should remove 
> > the .pngs now and require qtsvg at configure time.
> 
> Sure, all pngs which have a svg counterpart can be removed as they will
> never be used. BTW, isn't QtSvg already required at configure time?
> 
> > For me personally the svgs work fine both with qt 4.8 and 5.3, so I would 
> > not have a problem with that. I only thought there might by exotic systems 
> > where it does not work, and therefore I thought it might be a good idea to 
> > ship both png and svg for one release, instead of switching from only png 
> > directly to only svg.
> 
> I also use "exotic" systems that are not officially supported by Qt
> (which doesn't even compile cleanly without patching the sources) and
> never had a problem with svgs. The only problem that I can foresee is
> that Qt is built without zlib support (and this has to be done
> also deliberately excluding the zlib version that Qt would include in
> the absence of a system version), in which case compressed svgs cannot
> be read, or that Qt is statically built, in which case the qsvg plugin
> must be explicitly included when compiling. Note that this is not the
> QtSvg library, but the plugin in the plugins/imageformats directory.

There was an issue on Windows that came from not including a .dll. Now
that the issue has been fixed, we should decide if we would like to
consider removing the .pngs. If we would like to remove the .pngs for
2.2.0 I think we should definitely do this for beta1. Conversely, if we
remove the .pngs for beta, I think it would still be OK to add them back
for the final release if we discover problems, as JMarc proposed [1].

Thoughts?

[1]
https://www.mail-archive.com/search?l=mid=5648B9FA.3070705%40lyx.org


signature.asc
Description: PGP signature

Re: We now include both png and svgz?

[Enrico Forestieri]

On Sun, Nov 15, 2015 at 05:57:00PM +0100, Georg Baum wrote:

> Enrico Forestieri wrote:
> 
> > It is not intended to work that way. If the svgz image is there, it is
> > not expected that Qt is not able to show it. The double entry are there
> > only to assure that if there is no svgz then the png can be used instead.
> > In this way, you are able to override the default icons by placing your
> > own in the ~/.lyx/images directory, even if they are pngs.
> 
> OK, if it is assumed that qt can show svg images, then it does not make 
> sense to ship or own .pngs. If we want to stick with this, we should remove 
> the .pngs now and require qtsvg at configure time.

Sure, all pngs which have a svg counterpart can be removed as they will
never be used. BTW, isn't QtSvg already required at configure time?

> For me personally the svgs work fine both with qt 4.8 and 5.3, so I would 
> not have a problem with that. I only thought there might by exotic systems 
> where it does not work, and therefore I thought it might be a good idea to 
> ship both png and svg for one release, instead of switching from only png 
> directly to only svg.

I also use "exotic" systems that are not officially supported by Qt
(which doesn't even compile cleanly without patching the sources) and
never had a problem with svgs. The only problem that I can foresee is
that Qt is built without zlib support (and this has to be done
also deliberately excluding the zlib version that Qt would include in
the absence of a system version), in which case compressed svgs cannot
be read, or that Qt is statically built, in which case the qsvg plugin
must be explicitly included when compiling. Note that this is not the
QtSvg library, but the plugin in the plugins/imageformats directory.

-- 
Enrico

Re: We now include both png and svgz?

[Enrico Forestieri]

On Sun, Nov 15, 2015 at 05:31:28PM -0500, Scott Kostyshak wrote:
> 
> Is the only disadvantage of including the .pngs bloat in the sense that
> we are increasing the size of our tars and of the repos?

Yep.

> Or is there a
> concern that shipping .pngs could lead to different issues also (because
> our code could get confused)?

This should not be the case. What may happen is that someone places a
png in ~/.lyx/images, in which case this png will be picked up instead
of the svg version. But this is expected. The rule is that when LyX
searches for icons, the user directory is first looked up, then the
system directory and lastly the resource system. For each directory, a
svg image has precedence. Failing to find svgz or png images, what was
stored in the resource system is used.

This also reminds me that the resource system has to be updated for
storing the svgs instead of the pngs.

-- 
Enrico

Re: We now include both png and svgz?

[Scott Kostyshak]

On Sun, Nov 15, 2015 at 05:59:38PM +0100, Jean-Marc Lasgouttes wrote:
> Le 15/11/2015 17:57, Georg Baum a écrit :
> >OK, if it is assumed that qt can show svg images, then it does not make
> >sense to ship or own .pngs. If we want to stick with this, we should remove
> >the .pngs now and require qtsvg at configure time.
> >
> >For me personally the svgs work fine both with qt 4.8 and 5.3, so I would
> >not have a problem with that. I only thought there might by exotic systems
> >where it does not work, and therefore I thought it might be a good idea to
> >ship both png and svg for one release, instead of switching from only png
> >directly to only svg.
> 
> Or we only ship svg for our beats and see what happens. We could decide to
> re-introduce pngs if needed.
> 
> In any case, it is good to have the code for pngs still working.

Is the only disadvantage of including the .pngs bloat in the sense that
we are increasing the size of our tars and of the repos? Or is there a
concern that shipping .pngs could lead to different issues also (because
our code could get confused)?

Scott


signature.asc
Description: PGP signature

Re: We now include both png and svgz?

[Georg Baum]

Enrico Forestieri wrote:

> It is not intended to work that way. If the svgz image is there, it is
> not expected that Qt is not able to show it. The double entry are there
> only to assure that if there is no svgz then the png can be used instead.
> In this way, you are able to override the default icons by placing your
> own in the ~/.lyx/images directory, even if they are pngs.

OK, if it is assumed that qt can show svg images, then it does not make 
sense to ship or own .pngs. If we want to stick with this, we should remove 
the .pngs now and require qtsvg at configure time.

For me personally the svgs work fine both with qt 4.8 and 5.3, so I would 
not have a problem with that. I only thought there might by exotic systems 
where it does not work, and therefore I thought it might be a good idea to 
ship both png and svg for one release, instead of switching from only png 
directly to only svg.



Georg

Re: We now include both png and svgz?

[Jean-Marc Lasgouttes]

Le 15/11/2015 17:57, Georg Baum a écrit :

OK, if it is assumed that qt can show svg images, then it does not make
sense to ship or own .pngs. If we want to stick with this, we should remove
the .pngs now and require qtsvg at configure time.

For me personally the svgs work fine both with qt 4.8 and 5.3, so I would
not have a problem with that. I only thought there might by exotic systems
where it does not work, and therefore I thought it might be a good idea to
ship both png and svg for one release, instead of switching from only png
directly to only svg.


Or we only ship svg for our beats and see what happens. We could decide 
to re-introduce pngs if needed.


In any case, it is good to have the code for pngs still working.

JMarc

Re: We now include both png and svgz?

[Georg Baum]

Vincent van Ravesteijn wrote:

> In any case, the logic is a bit strange. The icon names are defined
> with the extension {png,svgz} so that any one matched, and getPixmap
> checks whether the file can be loaded or not. However, iconName then
> already decides on whether it is png or svgz just based on existence,
> not on whether it can be loaded.
> 
> So, either we make QtSvg a "required" dependency, and cmake/autotools
> should check for it, or we improve the logic to decide whether we lood
> the png or svgz variant.

For 2.2 I'd prefer to improve the logic, since it is the first release which 
includes the svg icons. For 2.3 we can get rid of the .pngs.

Does the attached patch help?


Georgdiff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp
index 5e213e6..661a540 100644
--- a/src/frontends/qt4/GuiApplication.cpp
+++ b/src/frontends/qt4/GuiApplication.cpp
@@ -156,12 +156,20 @@ using namespace std;
 using namespace lyx::support;
 
 
+static bool enableSVG = false;
 static void initializeResources()
 {
 	static bool initialized = false;
 	if (!initialized) {
 		Q_INIT_RESOURCE(Resources);
 		initialized = true;
+		QList qt_formats = QImageReader::supportedImageFormats();
+		for (QList::const_iterator it = qt_formats.begin();
+		 it != qt_formats.end() && !enableSVG; ++it) {
+			string format = ascii_lowercase((const char *) *it);
+			if (format == "svg")
+enableSVG = true;
+		}
 	}
 }
 
@@ -520,13 +528,14 @@ QString iconName(FuncRequest const & f, bool unknown)
 	QStringList imagedirs;
 	imagedirs << "images/" << "images/ipa/";
 	search_mode mode = theGuiApp()->imageSearchMode();
+	QString const formats = (enableSVG ? "svgz,png" : "png");
 	for (int i = 0; i < imagedirs.size(); ++i) {
 		QString imagedir = imagedirs.at(i) + path;
-		FileName fname = imageLibFileSearch(imagedir, name1, "svgz,png", mode);
+		FileName fname = imageLibFileSearch(imagedir, name1, formats, mode);
 		if (fname.exists())
 			return toqstr(fname.absFileName());
 
-		fname = imageLibFileSearch(imagedir, name2, "svgz,png", mode);
+		fname = imageLibFileSearch(imagedir, name2, formats, mode);
 		if (fname.exists())
 			return toqstr(fname.absFileName());
 	}
@@ -537,12 +546,12 @@ QString iconName(FuncRequest const & f, bool unknown)
 		LYXERR0("Directory " << path << " not found in resource!");
 		return QString();
 	}
-	if (res.exists(name1 + ".svgz"))
+	if (enableSVG && res.exists(name1 + ".svgz"))
 		return path + name1 + ".svgz";
 	else if (res.exists(name1 + ".png"))
 		return path + name1 + ".png";
 
-	if (res.exists(name2 + ".svgz"))
+	if (enableSVG && res.exists(name2 + ".svgz"))
 		return path + name2 + ".svgz";
 	else if (res.exists(name2 + ".png"))
 		return path + name2 + ".png";
@@ -557,7 +566,7 @@ QString iconName(FuncRequest const & f, bool unknown)
 
 	if (unknown) {
 		QString imagedir = "images/";
-		FileName fname = imageLibFileSearch(imagedir, "unknown", "svgz,png", mode);
+		FileName fname = imageLibFileSearch(imagedir, "unknown", formats, mode);
 		if (fname.exists())
 			return toqstr(fname.absFileName());
 		return QString(":/images/unknown.png");
@@ -587,17 +596,19 @@ bool getPixmap(QPixmap & pixmap, QString const & path)
 QPixmap getPixmap(QString const & path, QString const & name, QString const & ext)
 {
 	QString imagedir = path;
-	FileName fname = imageLibFileSearch(imagedir, name, ext, theGuiApp()->imageSearchMode());
-	QString fpath = toqstr(fname.absFileName());
-	QPixmap pixmap = QPixmap();
-
-	if (getPixmap(pixmap, fpath)) {
-		return pixmap;
-	}
-	
 	QStringList exts = ext.split(",");
-	fpath = ":/" + path + name + ".";
 	for (int i = 0; i < exts.size(); ++i) {
+		if (!enableSVG && (exts.at(i) == "svg" || exts.at(i) == "svgz"))
+			continue;
+		FileName fname = imageLibFileSearch(imagedir, name, ext.at(i),
+		theGuiApp()->imageSearchMode());
+		QString fpath = toqstr(fname.absFileName());
+		QPixmap pixmap = QPixmap();
+
+		if (getPixmap(pixmap, fpath))
+			return pixmap;
+
+		fpath = ":/" + path + name + ".";
 		if (getPixmap(pixmap, fpath + exts.at(i))) {
 			return pixmap;
 		}

Re: We now include both png and svgz?

[Enrico Forestieri]

On Thu, Nov 12, 2015 at 09:14:02PM +0100, Georg Baum wrote:
> Vincent van Ravesteijn wrote:
> 
> > In any case, the logic is a bit strange. The icon names are defined
> > with the extension {png,svgz} so that any one matched, and getPixmap
> > checks whether the file can be loaded or not. However, iconName then
> > already decides on whether it is png or svgz just based on existence,
> > not on whether it can be loaded.
> > 
> > So, either we make QtSvg a "required" dependency, and cmake/autotools
> > should check for it, or we improve the logic to decide whether we lood
> > the png or svgz variant.
> 
> For 2.2 I'd prefer to improve the logic, since it is the first release which 
> includes the svg icons. For 2.3 we can get rid of the .pngs.
> 
> Does the attached patch help?

Note that Qt4 can deal perfectly fine with svgz. The only ways it
can fail are 1) the library was compiled without support for zlib;
2) the qsvg plugin is missing; 3) the library was compiled statically.

For case 1, the attached patch might help (I think it requires that
gunzip is in the path).
For case 2, simply put the qsvg.dll plugin where Qt expects it.
For case 3, the plugin is only available through a static library and
one needs to tweak the LyX sources for linking it. It suffices adding
the following two lines to (for example) main.cpp

#include 
Q_IMPORT_PLUGIN(qsvg)

and then adding the libqsvg static library to the final link command.

-- 
Enrico
diff --git a/src/frontends/qt4/GuiApplication.cpp 
b/src/frontends/qt4/GuiApplication.cpp
index 5e213e6..51335a1 100644
--- a/src/frontends/qt4/GuiApplication.cpp
+++ b/src/frontends/qt4/GuiApplication.cpp
@@ -569,7 +569,13 @@ QString iconName(FuncRequest const & f, bool unknown)
 
 bool getPixmap(QPixmap & pixmap, QString const & path)
 {
-   if (pixmap.load(path)) {
+#if QT_VERSION < 0x05 && defined(Q_OS_WIN) 
+QString const fpath = path.endsWith(".svgz")
+   ? toqstr(unzipFile(FileName(fromqstr(path))).absFileName()) : path;
+#else
+QString const & fpath = path;
+#endif
+   if (pixmap.load(fpath)) {
 #if QT_VERSION >= 0x05
if (path.endsWith(".svgz") || path.endsWith(".svg") ) {
GuiApplication const * guiApp = theGuiApp();

Re: We now include both png and svgz?

[Enrico Forestieri]

On Wed, Nov 11, 2015 at 01:37:51PM +0100, Vincent van Ravesteijn wrote:
> 
> So, either we make QtSvg a "required" dependency, and cmake/autotools
> should check for it, or we improve the logic to decide whether we lood
> the png or svgz variant.

It is not intended to work that way. If the svgz image is there, it is
not expected that Qt is not able to show it. The double entry are there
only to assure that if there is no svgz then the png can be used instead.
In this way, you are able to override the default icons by placing your
own in the ~/.lyx/images directory, even if they are pngs.

-- 
Enrico

Re: We now include both png and svgz?

[Vincent van Ravesteijn]

On Wed, Nov 11, 2015 at 1:02 PM, Jean-Marc Lasgouttes
 wrote:
> Le 10/11/2015 10:02, Vincent van Ravesteijn a écrit :
>>
>> At the moment you cannot use LyX with Qt4 because it cannot read svgz
>> files, and the code is not correctly falling back to png files.
>>
>> So yes, we need both, otherwise we will have to require newer versions of
>> Qt.
>
>
> My copy of lyx 2.2 with qt4.8 handles svg icons fine. Could it be a
> cmake/qt4 issue? (I have no idea how cmake checking works). The difference
> is only whether we link against QtSvg, I think.
>
> JMarc
>

That could be yes.

In any case, the logic is a bit strange. The icon names are defined
with the extension {png,svgz} so that any one matched, and getPixmap
checks whether the file can be loaded or not. However, iconName then
already decides on whether it is png or svgz just based on existence,
not on whether it can be loaded.

So, either we make QtSvg a "required" dependency, and cmake/autotools
should check for it, or we improve the logic to decide whether we lood
the png or svgz variant.

Vincent

Re: We now include both png and svgz?

[Jean-Marc Lasgouttes]

Le 10/11/2015 10:02, Vincent van Ravesteijn a écrit :

At the moment you cannot use LyX with Qt4 because it cannot read svgz
files, and the code is not correctly falling back to png files.

So yes, we need both, otherwise we will have to require newer versions of Qt.


My copy of lyx 2.2 with qt4.8 handles svg icons fine. Could it be a 
cmake/qt4 issue? (I have no idea how cmake checking works). The 
difference is only whether we link against QtSvg, I think.


JMarc

Re: We now include both png and svgz?

[Kornel Benko]

Am Mittwoch, 11. November 2015 um 13:02:53, schrieb Jean-Marc Lasgouttes 

> Le 10/11/2015 10:02, Vincent van Ravesteijn a écrit :
> > At the moment you cannot use LyX with Qt4 because it cannot read svgz
> > files, and the code is not correctly falling back to png files.
> >
> > So yes, we need both, otherwise we will have to require newer versions of 
> > Qt.
> 
> My copy of lyx 2.2 with qt4.8 handles svg icons fine. Could it be a 
> cmake/qt4 issue? (I have no idea how cmake checking works). The 
> difference is only whether we link against QtSvg, I think.
> 
> JMarc

No, I have the same experience here. I had only to install the appropriate
Qt4 package. (Ubuntu: libqt4-svg)

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: We now include both png and svgz?

[Jean-Marc Lasgouttes]

Le 11/11/15 13:37, Vincent van Ravesteijn a écrit :

In any case, the logic is a bit strange. The icon names are defined
with the extension {png,svgz} so that any one matched, and getPixmap
checks whether the file can be loaded or not. However, iconName then
already decides on whether it is png or svgz just based on existence,
not on whether it can be loaded.

So, either we make QtSvg a "required" dependency, and cmake/autotools
should check for it, or we improve the logic to decide whether we lood
the png or svgz variant.


We could indeed make svg a requirement. Would there be drawbacks?

JMarc

Re: We now include both png and svgz?

[Vincent van Ravesteijn]

On Tue, Nov 10, 2015 at 4:13 AM, Scott Kostyshak  wrote:
> When experimenting with building the tar balls, I noticed a significant
> difference in size between 2.2.0dev and 2.1.4.
>
> lyx-2.1.4.tar.gz is 20.8 MB
> lyx-2.2.0dev.tar.gz is 24.8 MB
>
> A quick check seems to show that most of the change comes from now
> including .svgz (in addition to .png). Why do we include both?
>
> I just wanted to check that this is expected.
>
> Scott

At the moment you cannot use LyX with Qt4 because it cannot read svgz
files, and the code is not correctly falling back to png files.

So yes, we need both, otherwise we will have to require newer versions of Qt.

Vincent

Re: We now include both png and svgz?

[Pavel Sanda]

Scott Kostyshak wrote:
> > At the moment you cannot use LyX with Qt4 because it cannot read svgz
> > files, and the code is not correctly falling back to png files.
> > 
> > So yes, we need both, otherwise we will have to require newer versions of 
> > Qt.
> 
> I see. Makes sense. I will set a note to myself that we should remove
> the pngs if we ever require Qt 5.

Sounds like bug with target 2.3. P

Re: We now include both png and svgz?

[Scott Kostyshak]

On Tue, Nov 10, 2015 at 10:02:15AM +0100, Vincent van Ravesteijn wrote:
> On Tue, Nov 10, 2015 at 4:13 AM, Scott Kostyshak  wrote:
> > When experimenting with building the tar balls, I noticed a significant
> > difference in size between 2.2.0dev and 2.1.4.
> >
> > lyx-2.1.4.tar.gz is 20.8 MB
> > lyx-2.2.0dev.tar.gz is 24.8 MB
> >
> > A quick check seems to show that most of the change comes from now
> > including .svgz (in addition to .png). Why do we include both?
> >
> > I just wanted to check that this is expected.
> >
> > Scott
> 
> At the moment you cannot use LyX with Qt4 because it cannot read svgz
> files, and the code is not correctly falling back to png files.
> 
> So yes, we need both, otherwise we will have to require newer versions of Qt.

I see. Makes sense. I will set a note to myself that we should remove
the pngs if we ever require Qt 5.

Scott

Re: We now include both png and svgz?

[Scott Kostyshak]

On Tue, Nov 10, 2015 at 06:42:50PM -0800, Pavel Sanda wrote:
> Scott Kostyshak wrote:
> > > At the moment you cannot use LyX with Qt4 because it cannot read svgz
> > > files, and the code is not correctly falling back to png files.
> > > 
> > > So yes, we need both, otherwise we will have to require newer versions of 
> > > Qt.
> > 
> > I see. Makes sense. I will set a note to myself that we should remove
> > the pngs if we ever require Qt 5.
> 
> Sounds like bug with target 2.3. P

Good idea. Done at #9857.

Scott