KeyWorksRW
Investigate possible way to use output from commercial SVG editors
Description:
This issue revolves around the problem that SVG from most editors cannot be displayed correctly, if at all in the nanosvg code that wxWidgets use. It is a major problem for wxUiEditor which has 156 png files, the vast majority of which need to be recreated as SVG files. While that's probably more than most other projects need to deal with, still it's going to affect anyone with toolbars that want to create an image once for all resolutions, rather than creating multiple versions for each resolution, and then still have the image look poorly if the user isn't using an exact match of the resolutions that the images were created for.
nanosvg had the advantage of being easy to add to wxWidgets. However, not only is the support for SVG quite limited, but it is not actively maintained, so there's no hope of it being expanded to support the full ruange of standard SVG files.
While InkScape has the ability to convert monochrome bitmaps, that's not much help for use with colored images. Since nonosvg doesn't correctly display SVGs from the professional designers from Adobe and Affinity, I've been left with InkScape as the most reliable option. By "most reliable" I'm referring to the SVG it produces -- the program itself crashes frequently for me, losing all the work I've done since the last manual save.
I own Affinity Designer, and it does produce SVG files that InkScape can load (though saving them from InkScape still produces a file that nanosvg cannot display correctly). wxUiEditor already parses SVG files in order to remove all the unused content from InkScape generated files (typically reducing the file size by 85%). I'd like to at least look at what Affinity Designer is producing to see if I can tweak it's output to produce something that nanosvg can display correctly.
Github doesn't give any information about forks other than listing the link to it. There are currently 295 forks for nanosvg, and 43 unmerged pull requests in the main repository. It might be useful to create a clone of nanosvg and merge the pull requests into it to see if any of those changes resolves the problem. Most of the forks won't have useful changes, but some of them might be worth looking at. Should these changes plus anything additional that I can resolve from looking at the Affinity Designer output, then I might at least be able to get something that works for wxUiEditor, and if it remains a simple library, then I might be able to make the library available as a standalone repository that other people could use.
nanosvg repository is at https://github.com/memononen/nanosvg -- it uses a zlib license so unlike a GNU license, we could use the code without requiring all the licenses in the repository to be changed.
Have you ever considered this library ? https://github.com/sammycage/lunasvg
I had looked at several libraries, but not this one. I see there is an old PR that shows how to hook it up to wxWidgets. Doesn't look like the PR will be accepted, but the code example would be useful. No idea if this can parse commercial SVG files, but unlike nanosvg, this one is at least being maintained. I'll definitely look into it -- thanks for the tip!
I've been using it since the last sommer, the author is very responsive and kind The library is well written and without any external dependencies, except for the use of a call to this library stb in case you want to save the image
I had the opportunity to test it with InkScape files and other uge SVG files I found random on internet; it works They are working even on texts
Using a few rows of code, it easy to obtain "dynamic" bitmaps within your own application transformations scale
I added a dialog to a test app I have where I used wxBitmapBundle::FromSVGFile() to load an SVG file and display in one wxStaticBitmap, and then used lunasvg::Document::loadFromFile() to load it again using LunaSVG and than ran it through a function I wrote to convert it to a wxImage. I then used that to display in a second wxStaticBitmap so that I could view them side by side.
After verifying that it worked the same on SVG files that wxWidgets can display, I started loading up a much larger variety of svg files. I tried a few from Affinity Designer -- these displayed correctly with LunaSVG, and none of the ones I tried displayed correctly using nanosvg (wxWidgets). Loading and saving these via InkScape did not resolve the probelm with nanosvg. I also tried some SVG files from the InkScape repository and the Rust installation, and while some of them displayed correctly in nanosvg, the ones that didn't displayed fine in LunaSVG.
I did run into a couple of SVG files that LunaSVG did not completely display correctly compared to nanosvg, though I didn't research it enough to find out why.
I'm not seeing any SVG issues raised in the wxWidgets repository, though I suspect that's partly because not a lot of people are using SVG files with wxWidgets yet. There is an open issue about using ThorVG which has more capability then LunaSVG (e.g., it supports Text) but would be a lot harder to integrate with wxWidgets.
Short term, using this to convert the SVG files used to build wxUiEditor would be very helpful since I could then use Affinity Designer to create replacements for the 162 png files that are currently in use. Even InkScape can create SVG files that nanosvg doesn't display correctly, and I do not want to be spending a bunch of time creating SVG files only to have them non-displayable.
Long term, it would be potentially useful to see if I can't create a render2Image() function in LunaSVG::Document -- currently I render a bitmap, and then have to convert that bitmap pixel by pixel to a format that wxImage will support. It works, but it would be useful to skip that intermediate data.
Kudos to LunaSVG for create a SVG rendering library that is very easy to build, actively maintained, and is capable of displaying a much wider variety of SVG files than nanosvg!
Thank you @Randalphwa for sharing
Honestly speaking I've never used nanovg for my job. Talking about SVG file integration, I'd consider it no more than a kind of "work-around". I'm even wonder why it has been used by wxWidgets maintainers.
Among other libraries which I had the opportunity to verify, LunaSVG is the one still I'm considering usefull, extremly ligtht, stand alone, well written in a modern C++ and very easy to integrate. I had the opportunity to chat with the main developer @sammycage, which is very kind and shows high skills in programming and listening. Just a few days ago he has implemented high level functions to modify all SVG attributes. Please see this issue [ accessing the internal objects ] Thanks to such features you are able to "animate" the SVG image during runtime operations. He even started to implement transformations on local objects ... I would suggest to contact directly @sammycage in order to talk with him regarding your "concerns".
This is the code I use to convert from bitmap to wxImage
unsigned char *png = stbi_write_png_to_mem((unsigned char *)bitmap.data(), 0, (int)bitmap.width(), (int)bitmap.height(), 4, &len);
wxMemoryInputStream stream(png, len);
// To image
wxImage img(stream, wxBITMAP_TYPE_PNG);
The function stbi_write_png_to_mem is provided by the library itself and it belongs to this repository [ https://github.com/nothings/stb ]
Anyway even this topic could be discussed easily with the LunaSVG developer.
Talking about wxWidgets and SVG files I think they should get rid of nano SVG, unless there are some planning to refurbish it. Perhaps you could suggest them the use LunaSVG :)
The problem with stbi_write_png_to_mem is that you are running it through zlib compression, and then immediately turning around and decompressing it. That may be fine for a small image and/or just a few images, but wxUiEditor has over 175 imgages that ultimately need to be recreated as SVG files, and I would worry about the performance of that.
What I use is the following:
wxImage ConvertARGB32PremultipliedToWxImage(const lunasvg::Bitmap& lunaBitmap)
{
int width = lunaBitmap.width();
int height = lunaBitmap.height();
wxImage wxImg(width, height);
if (!wxImg.HasAlpha())
wxImg.InitAlpha();
const unsigned char* pixels = lunaBitmap.data();
for (int y = 0; y < height; ++y)
{
for (int x = 0; x < width; ++x)
{
int i = (y * width + x) * 4;
unsigned char a = pixels[i + 3];
if (a == 0)
{
wxImg.SetRGB(x, y, 0, 0, 0);
wxImg.SetAlpha(x, y, 0);
}
else
{
unsigned char r = (pixels[i + 2] * 255) / a;
unsigned char g = (pixels[i + 1] * 255) / a;
unsigned char b = (pixels[i + 0] * 255) / a;
wxImg.SetRGB(x, y, r, g, b);
wxImg.SetAlpha(x, y, a);
}
}
}
return wxImg;
}
That makes this code possible:
auto svg = lunasvg::Document::loadFromFile(filename.ToStdString());
auto bitmap = svg->renderToBitmap(FromDIP(size.x), FromDIP(size.y));
if (bitmap.valid())
{
auto image = ConvertARGB32PremultipliedToWxImage(bitmap);
m_bmp_luna_svg->SetBitmap(image);
where m_bmp_luna_svg is a wxStaticBitmap. This avoids pulling in another zlib library, as well as avoiding the compression/decompression step. (And yes, you are welcome to use that function if you wish).
I do intend to suggest LunaSVG as an alternative to wxWidgets after I've actually merged it in as a private modification so that I can submit is as a PR. A lot of work to be done first -- I need test cases, benchmarks, etc. plus of course integrating it as closely as nanosvg is now.
thank you @Randalphwa for sharing your funcion ... good luck for your suggestion you are going to introduce to wx team
Perhaps you already knew, but LunaSVG has got the following functions to "transform" a SVG image: Rotate, Flip X, Flip Y, Skew and Scale Please see this issue
I tried building the sources for lunasvg directly into one of my test project which resulted in 60+ compiler warnings. It turns out that most class members do not use any kind of prefix to indicate it's a class member. That means if you read a line of code like:
newState.canvas = Canvas::create(0, 0, width, height);
You do not know if width and height are the class member or a local variable. If the local variable is defined just before it's used you might notice -- or in my case my editor shows it to me along with the warning "declaration of width hides class member.
There's no way I could recommend adding a 3rd party library that will generate 60+ warnings to anyone that compiles wxWidgets with /W4 level warnings (or worse, warnings as errors). I also agree with this warning -- it's incredibly confusing to to not be able to immediately know whether a variable is a class member, or it's being overridden by a local variable.
Unfortunately because he used common names for both class members and local variables, you can't use any kind of global search and replace to add prefixes to the class variables. It would be a major headache to fix this at this point in the project.
@Randalphwa you are right its a pity, but before giving up with Luna SVG let me try to comunicate with the develeper ... Up to now I superseeded this "anomaly" in favour of nice features and library caracteristic itsself, then I built the library as a static one and it has been wrapped in another class written for my needs. Indeed thinking of it as part of wxWidgets it should be a bit reviewed.
The problem is that it's mostly the public: class members which are declared without a prefix. That means if you change them now, you risk breaking anyone who uses those members directly. I don't see any way around changing the public: member names without spinning off a new version (LunaSVG2 ?). At some point I'll probably create a fork and try doing exactly that to see how much of an effort it would actually be. But that still wouldn't solve the problem of breaking anyone that accessed those public class members without a completely new version.
I had a chat this mornign with @sammycage and based on what he wrote he is going to change such public interface ... Before moving a finger :), I'd wait for a while observing the new library asset I'm sure @sammycage is able to provide.
@Randalphwa It's worth noting that defining public class members without a prefix is a common practice, enhancing convenience for direct use without setter and getter functions in trivial classes where direct modification won't cause harm. @rossanoparis While considering renaming, it's crucial to weigh potential impact on existing users and explore alternative solutions for maintaining compatibility.
@sammycage The most critical issue for me is that local variables are being defined with the same name as the class members. A non-breaking option would be to rename local variables whenever the name conflicts with a class member. That also ensures that it wasn't a mistake to use the local variable instead of the class member, while still maintaining compatibility for anyone accessing the class members directly.
@Randalphwa I've grasped your point now. Could you please share your warning log with me? My IDE/compiler isn't providing any feedback on such warnings.
Case 1:
class A {
public:
void func() {
int b = a * 1;
int a = b * 1;
}
int a;
};
Case 2:
class B {
public:
B(int b) : b(b) {}
int b;
};
Case 3
class C {
public:
void func() {
int c = this->c;
c += 5;
}
int c;
};
Case 4
void d() {
int d = 0;
{
int d = 1;
}
}
@Randalphwa After a thorough review, it appears that the latest commit has successfully addressed the issues raised in case 1. However, I would like to inquire about the status of case 2, case 3, and case 4. Could you kindly provide insights into whether these cases still generate warnings or if there have been any recent updates addressing these concerns? Your efforts in resolving these matters are greatly appreciated.
@sammycage, building the latest code from master branch I get 77 warnings. If you could solve them it would be really nice
@sammycage I've already got a fork of your project. Since my compiler is providing me with all of these warnings, I'd be willing to create a PR with fixes -- casting for the "possible loss of data" warning, and renaming local variables that conflict with class members. My guess is you would want to change some of the local names I used, so you could use my PR as a starting point to change as desired. It would also need a careful code review -- since it would be easy to mistakenly not update every use of the new variable name, and accidentally use the class member instead.
@rossanoparis I've just pushed an update to the master branch that addresses the warnings you mentioned. Could you please check and confirm if any warnings are still lingering?
@sammycage I only encountered two warnings compiling using MSVC with /W4 and cxx_std_20:
C:\rwCode\lunasvg\source\layoutcontext.cpp(162,10): warning C4458: declaration of 'transform' hides class member C:\rwCode\lunasvg\source\layoutcontext.h(146,15): note: see declaration of 'lunasvg::LayoutMarker::transform' C:\rwCode\lunasvg\source\layoutcontext.cpp(178,10): warning C4458: declaration of 'transform' hides class member C:\rwCode\lunasvg\source\layoutcontext.h(146,15): note: see declaration of 'lunasvg::LayoutMarker::transform'
@sammycage The next question is whether you are okay with having lunasvg added as a submodule to the wxWidgets repository (https://github.com/wxWidgets/wxWidgets)? I already have it building in a private build, and I'm not seeing anything that would look problematic in terms of full integration. I.e., I'm not seeing any need for changes on your end other then fixing the warnings which you have already taken care of (once those last two are dealt with). I've spent the afternoon comparing hundreds of SVG files from a wide variety of sources, comparing them with the nanosvg that wxWidgets currently uses and lunasvg -- I have yet to encounter anything that lunasvg doesn't display correctly and several dozen that nanosvg does not display correctly. As such, with your permission I'll raise the possibility of incorporating lunasvg as an alternative to nanosvg in the wxWidgets code base.
@Randalphwa @rossanoparis I appreciate your thorough evaluation and efforts in integrating Lunasvg into the wxWidgets repository. I'm pleased to confirm that the last two warnings have been successfully fixed, and I am supportive of integrating Lunasvg as a submodule to the wxWidgets repository.
Feel free to proceed with proposing the inclusion of Lunasvg as an alternative to nanosvg in the wxWidgets codebase. If there are any additional steps or considerations needed from my end, please let me know. I trust this decision will contribute to the overall improvement of SVG rendering within wxWidgets.
@sammycage @sammycage
Feature issue submitted: https://github.com/wxWidgets/wxWidgets/issues/24216
I will definitely be integrating this into wxUiEditor whether it ends up being part of wxWidgets or not. There is absolutely no way I would replace the 170+ png files that wxUiEditor currently uses with SVG files without be certain that they would display correctly. So, thank-you @rossanoparis for drawing my attention to lunasvg, and thanks to @sammycage for creating such a reliable SVG rendering engine!
PR #1395 incorporates a private branch of wxWidgets that made it possible to replace nanosvg with lunasvg which I have now done in wxUiEditor. That should show up in daily builds starting on 2/3. The relevant changes to wxWidgets are primarily in wxWidgets/src/generic/bmpsvg.cpp along with 3rdparty/lunasvg.
Once sammycage decides whether or not to merge the PR I made to lunasvg, I'll finish up the wxWidgets branch and submit the changes to the wxWidgets team.
As a side note, I have replaced most of the existing SVG images with ones edited in a commercial designer, and they worked flawlessly. Now I just need to recreate the remaining 150+ PNG images as SVG files... 🙁
Also note that because wxUiEditor now uses lunasvg for the Mockup panel, it's quite possible for an SVG file to look right in wxUiEditor and wrong in the user's application. For wxPython and wxRuby users there won't be anything they can do until those languages switch to a newer version of wxWidgets. C++ users will have the option of updating their wxWidgets sources -- but wxWidgets 3.3 doesn't have a release date set at this point, and I'm not planning on trying to support lunasvg on wxWidgets 3.2.
Thank you @Randalphwa for this update, your work on wxUiEditor is comforting. I had to suspend my tests regarding DialogBlocks to wxUiEditor porting due to our working skedule. I've planned such activity in April ... Anyway I wish you all the best
LunaSVG is integrated into wxUiEditor, with 164 SVG images now used for the UI. It still needs to have final integration into wxWidgets 3.3, but I'm not going to hold up the release of wxUiEditor 1.2.1 for that change. Hence, the milestone being removed.