Completely agree. I think the best solution for the plaintext icons is to get svg support compiled in the mingw64 qt pre-compiled binaries (I've already requested the mingw64 repo maintainer for this). Once I try it out myself, I'll probably go with modifying the Install.Win32 to instruct mingw64 not to use pacman qt5 binaries till they're fixed.
Sent: Saturday, May 07, 2016 at 3:36 AM
From: "Stephan Witt" <st.w...@gmx.net>
To: "Shankar Giri" <gir...@gmx.com>
Cc: "LyX Developers" <lyx-devel@lists.lyx.org>, "Scott Kostyshak" <skost...@lyx.org>
Subject: Re: SVG Icon going to fallback plaintext in Windows
From: "Stephan Witt" <st.w...@gmx.net>
To: "Shankar Giri" <gir...@gmx.com>
Cc: "LyX Developers" <lyx-devel@lists.lyx.org>, "Scott Kostyshak" <skost...@lyx.org>
Subject: Re: SVG Icon going to fallback plaintext in Windows
Am 07.05.2016 um 11:10 schrieb Shankar Giri <gir...@gmx.com>:
>
> Hi Stephan,
>
> Thanks for looking into this.
>
> I think we are mixing two bugs here (please correct me if I'm wrong).
>
> Bug 1: SVG converts poorly, so the effort has been to make the pictures look better. These are actually pictures in the LyX body, not the LyX toolbar icons themselves. I believe this is the bug you addressed in your latest patch.
The pixmaps converted by getPixmap() in GuiApplication are used as toolbar icons. To demonstrate it I’ve changed it to your version to produce the attached screen shot.
The code I’ve removed was introduced to get HiDPI icons with Qt 5.0 and other early versions. It is not useful anymore, AFAICS.
>
> Bug 2: Specifically for MinGW-W64 pacman repository built Qt libs, the pixmap.load(path); will fail for SVGs and SVGZs (built without SVG support). So icons all go blank and instead show fall-back text. My patch was aiming to only temporarily address Bug 2 not Bug 1.
>
> I'll double-check your latest patch on a Windows machine, but do let me know if my understanding is wrong.
The patch should change nothing there.
I wanted to explain the reasoning for the removal and why I don’t like your solution for all platforms.
Stephan
>
> Thanks!
> Shankar
>
> On 05/07/16 01:38, Stephan Witt wrote:
>> Am 07.05.2016 um 04:11 schrieb Shankar Giri Venkita Giri <gir...@gmx.com>:
>>> Hi Scott,
>>> Yes. The problem is the Qt5.6 binaries which I installed from the MSYS2+MINGW64 repository using the standard "pacman". The Qt5.6 libraries bundled with MINGW64 appear not to have built-in SVG image support(if we query the image formats supported, svg and svgz are not among them). Thus, it appears that we cannot load an svg or svgz into a pixmap for the icons like for eg. png or jpeg. But interestingly, the Qt5.6 QtSVG module/plugin did come with the install. So while we cannot open it natively, we can use the SVG module api to open SVG, render on an empty pixmap and send it on its way to the toolbar code. This is essentailly what my patch does.
>>> I'm guessing (don't know for sure) that the MSVC folks either a) compile and install Qt5.6 from source with SVG support enabled or b) they use a pre-compiled version of Qt5.6 that does have svg native support enabled. I filed a ticket to the MINGW64 package repository owner to try and update their packages with native svg image support, we'll see if that will eventually happen. But for mingw64 build users (my patch for the mingw64 build fixes recently got committed into 2.3-staging thanks to Georg, I suspect they will see the problem. I don't expect most users to perform lengthy Qt5 compilations - like me, they'd most likely just install from the pacman repo.
>>> I also mentioned this in the mingw ticket I filed because this problem appears specific to the mingw64 builds. http://www.lyx.org/trac/ticket/10053#comment:9
>>> I'm not quite sure how to proceed. My patch will work universally, I think (needs review), but it is not needed for other build platforms. I'll leave you guys to make the call.
>> Hi Shankar,
>>
>> I’ve just changed the code you provided a patch for. See ticket 10114 for the reasoning (http://www.lyx.org/trac/ticket/10114).
>>
>> The way you’ve solved the problem will definitively not produce good results on all platforms.
>> On HiDPI the icons are needed with much higher resolutions than 16x16.
>>
>> Regards,
>> Stephan
>
> Hi Stephan,
>
> Thanks for looking into this.
>
> I think we are mixing two bugs here (please correct me if I'm wrong).
>
> Bug 1: SVG converts poorly, so the effort has been to make the pictures look better. These are actually pictures in the LyX body, not the LyX toolbar icons themselves. I believe this is the bug you addressed in your latest patch.
The pixmaps converted by getPixmap() in GuiApplication are used as toolbar icons. To demonstrate it I’ve changed it to your version to produce the attached screen shot.
The code I’ve removed was introduced to get HiDPI icons with Qt 5.0 and other early versions. It is not useful anymore, AFAICS.
>
> Bug 2: Specifically for MinGW-W64 pacman repository built Qt libs, the pixmap.load(path); will fail for SVGs and SVGZs (built without SVG support). So icons all go blank and instead show fall-back text. My patch was aiming to only temporarily address Bug 2 not Bug 1.
>
> I'll double-check your latest patch on a Windows machine, but do let me know if my understanding is wrong.
The patch should change nothing there.
I wanted to explain the reasoning for the removal and why I don’t like your solution for all platforms.
Stephan
>
> Thanks!
> Shankar
>
> On 05/07/16 01:38, Stephan Witt wrote:
>> Am 07.05.2016 um 04:11 schrieb Shankar Giri Venkita Giri <gir...@gmx.com>:
>>> Hi Scott,
>>> Yes. The problem is the Qt5.6 binaries which I installed from the MSYS2+MINGW64 repository using the standard "pacman". The Qt5.6 libraries bundled with MINGW64 appear not to have built-in SVG image support(if we query the image formats supported, svg and svgz are not among them). Thus, it appears that we cannot load an svg or svgz into a pixmap for the icons like for eg. png or jpeg. But interestingly, the Qt5.6 QtSVG module/plugin did come with the install. So while we cannot open it natively, we can use the SVG module api to open SVG, render on an empty pixmap and send it on its way to the toolbar code. This is essentailly what my patch does.
>>> I'm guessing (don't know for sure) that the MSVC folks either a) compile and install Qt5.6 from source with SVG support enabled or b) they use a pre-compiled version of Qt5.6 that does have svg native support enabled. I filed a ticket to the MINGW64 package repository owner to try and update their packages with native svg image support, we'll see if that will eventually happen. But for mingw64 build users (my patch for the mingw64 build fixes recently got committed into 2.3-staging thanks to Georg, I suspect they will see the problem. I don't expect most users to perform lengthy Qt5 compilations - like me, they'd most likely just install from the pacman repo.
>>> I also mentioned this in the mingw ticket I filed because this problem appears specific to the mingw64 builds. http://www.lyx.org/trac/ticket/10053#comment:9
>>> I'm not quite sure how to proceed. My patch will work universally, I think (needs review), but it is not needed for other build platforms. I'll leave you guys to make the call.
>> Hi Shankar,
>>
>> I’ve just changed the code you provided a patch for. See ticket 10114 for the reasoning (http://www.lyx.org/trac/ticket/10114).
>>
>> The way you’ve solved the problem will definitively not produce good results on all platforms.
>> On HiDPI the icons are needed with much higher resolutions than 16x16.
>>
>> Regards,
>> Stephan
