Re: compilation error with Re: We now include both png and svgz?
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: compilation error with Re: We now include both png and svgz?
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?
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.
compilation error with Re: We now include both png and svgz?
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: We now include both png and svgz?
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
Re: We now include both png and svgz?
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?
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?
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?
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 \ + images/buffer-update_pdf5.svg
Re: We now include both png and svgz?
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?
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?
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&q=5648B9FA.3070705%40lyx.org signature.asc Description: PGP signature
Re: We now include both png and svgz?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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
Re: We now include both png and svgz?
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?
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?
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
We now include both png and svgz?
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