9 Improvements to SVG Export

This issue is being closed, and a new one is being opened that will include these same improvements plus additional ones.

OK, I thought you wanted me to close this issue and open a new one. Whatever works. I can reopen this one, if that's the way to go.

No, it's a good idea. This is a cool, easy to remember number. I'm still haggling with github support, and still awaiting some feedback on the set of 6 improvements. When all that is clear I'll update everything here. Thx.

This seems as good a place as any to post this for now: I deleted my entire MuseScore fork on GitHub, created a new fork, and cloned it to my client. Then in the github client I created a new branch. Then I opened the project in Qt Creator, ran the initial cmake, and now github client says I have 1567 changes, none of which are files I want to commit.
This is what happened to me previously, and how I took a wrong turn on the git highway. What should I do about all these changes to "compiled" files? Thanks in advance.

I think you mean to point me to the git workflow section of the handbook, don't you? here: https://musescore.org/en/developers-handbook/git-workflow

I'm following the compile instructions for sure, but now that I look at the git workflow instructions, I am not following those to the letter. I was hoping to avoid marrying myself to the git command line, but it looks like I'll have to make that commitment. If there's really something in the compile instructions you think I should look at, I will, but for now I'm going to start over with git and follow the command line workflow instructions to the letter. Thanks.

You're right, I missed that step. I was using MuseScore/build as the build directory because it already existed and seemed like the place to start. Thanks.

What I don't see is an instruction regarding the Win32 Install directory. Should that have a .qtc suffix too?

The Win32Install directory is mentioned, but I see no mention of .qtc suffix anywhere except the build directory. I searched the page for .qtc and qtc. I'm following the steps as we exchange these comments, so hopefully I'll be arriving to a correct state sometime soon.

I followed the instructions to the letter and now git looks more normal. Thanks for your assistance. If there's no feedback to my post on the other SVG topic over the next day or so, then I'll package all 6 changes and submit the pull request.

This is a complete reset of the original issue:
1) No more group wrappers for anything - There are now zero SVG <g> elements in the exported files. This reduces file size and greatly improves file readability.

2) Default values not written to the file - If an element's attribute value is the same as the default for that attribute, omit the attribute setting from the file (for that element). This reduces the file size noticeably and eliminates masses of repeated settings of defaults like fill="#000000".

3) class="TypeName" - Each element's type name (Element::Type as a string) is assigned to the standard SVG class attribute. This enables CSS and other forms of extensibility to the file and the score. SvgPaintEngine now has a member of type Element*, set from saveSVG() for each element being "painted". That is how the element type is known. Having this Element* available creates additional possibilities for future enhancements to the SVG output.

4) No more transforms - MuseScore exports SVG path and polyline elements. For all intents and purposes, version 2.0.2 draws all of those paths and polylines relative to the coordinates 0,0 then applies a transform attribute to position them on the canvas. The new code simplifies the resulting file by eliminating the transform attribute entirely and adjusting the coordinates of each path/polyline to the correct location on the canvas. A very simple change in the code, as all the data necessary is already available in all the right places.

5) Staff lines drawn once per system, not once per measure - This has the greatest impact on piano-roll-style scores or scores with extremely wide pages. MuseScore draws staff lines by measure internally. The new code simply consolidates all the measures for each system, and only draws one line per system. Smaller files, vastly improved readability of staff lines in the file.

6) 1:1 Scaling - In today's code, the scaling in file.cpp/saveSVG() scales to 300dpi, which is based on the SvgGenerator being a "printer" and this being a pdf-style print job. SVG is scalable, it doesn't require pre-scaling. So the mag variable in SaveSVG() is now always set to 1. It could be removed completely, but setting it to 1 is a safe first pass. It can be eliminated after it's been proven in users' hands for a while.

7) Two changes to the basic layout of the SVG document in order to improve consistency and simplify code.
a) <svg> element width and height attributes, units changed from mm to px - SVG default is px (pixels), the code is converting pixels to millimeters in order to set the value, so why bother converting? The resulting file should display in exactly the same size/scale as previously, but simpler code and more SVG-like SVG.
b) Continuing where 7a) leaves off, setting the width and height values to be more precisely the same as the viewBox values. They are the same in 2.0.2, but with different precision. The new code ignores the internal size variable and uses the viewBox values for everything, because they have more precision.

8) Three changes to normalize the code with the latest 72dpi efforts:
a) Initialization of the SvgPaintEnginePrivate class, change:
resolution = 72;
to
resolution = Ms::DPI;
b)saveSVG() - Delete the 2nd line of code, which sets this same variable to the same default value:
printer.setResolution(DPI)
c) SvgGenerator::metric() -It was using a literal 25.4 value in two places. For example, this line of code:
return qRound(d->engine->size().height() * 25.4 / d->engine->resolution());
is now:
return qRound(d->engine->size().height() / Ms::DPMM);

9) SvgGenerator::metric() again - This time it's a now previously "unhandled metric": PdmDevicePixelRatio. I have set the return value to 1 for this newly added metric, as that seems to be the baseline value, per the Qt Help docs. v2.0.2 generates multiple warnings because of this:
default:
qWarning("SvgGenerator::metric(), unhandled metric %d\n", metric);

And finally, as a bonus: 10) All the unused gradient and font-related code is now commented out, as are the unused members of the SvgPaintEngine attributes structure.

This staff lines code is easily removed if you so desire. The code is isolated to two places: 1) the initial loop that handles staff lines (file.cpp lines 2551-2564, and 2) the switch statement that then ignores staff lines (lines 2577-2580).

I've never even considered the possibility of an invisible measure like the example you posted, but the code could be modified to handle it inside that first loop. I don't know what a"cutaway" staff is. Is that when the last measure of a score doesn't fill the width of the page, because that case should be handled. As for changing the # of staff lines on a per measure basis, I thought I raised that issue prior to submitting the pull request and no one complained. But if it's necessary, this code does not handle it, as you point out.

In any case, the handling of those cases would take place in that initial loop. Let me know if I can be of assistance in any way, either removing the code or attempting to handle these cases.

I was just in the process of replying when I saw your newest post. Yes, I can see how to handle this stuff - it complicates an otherwise simple loop, but it shouldn't be a big deal. The different # of staff lines may be easy enough to fit in during the rest of this patch, as the code will now need to review properties of every measure in the system anyway. So checking for another property, # of lines instead of visibility, should be straightforward at that point.

I'll switch over to that issue and post code when I have it available. I was going to simply use the same old, now merged, branch in github, but it looks like I should create a new one with the new issue number. Am I correct about that?

My first thought about fixing this is:
> If (cutaway=TRUE for staff OR invisible measure in system) then
>> iterate by measure within this system
The code is currently iterating by page, by system. So this would maintain that, but within the "by system" loop it could operate by measure or not, depending on the circumstances.

That's a very simple, very complete fix, and once I figure out how to determine if the number of staff lines has changed, it will include that case with ease as well.

I was just about to post the question: "How do I know how many lines there are in the staff?" Currently the "lines" variable is private inside class StaffLines.

I've already got separate cases setup for the cutaway vs. visible - I can check a staff's cutaway without having to iterate over all the measures. But I'll check out the code link and consider doing it all by measure visibility in one case.

Thanks!

Oops - I have never used the capture tool. I'll check that out. Hopefully it's something simple that I forgot to connect.

Is there anywhere I could have found an appropriate list of test cases prior to making the pull request? Just trying to figure out how we might have avoided this extra churn after the merge.

Yes, I see this now. I need to update one block of code in fotomode.cpp. This should be an extremely simple fix.

Should I make two branches, one for each issue number here? That doesn't sound right to me. Please let me know how to manage that part on github. I should have these both issues fixed shortly. I'm currently working on a branch I've named: #92901-SVG-Invisible-Measure.

regarding cutaway vs. invisible: I agree and am in the process of working that very code. I'll post further details in the other issues' threads as I have the fixes posted to github.

Thanks for the info. Regarding staff lines: If you like, I can include code that checks for a change in the number of staff lines within a staff (within a system), even though that code will never execute until the UI feature is created. The code I have already committed to github for new issue #92901 could be easily adapted to add this feature internally now. That way it doesn't cause what I imagine to be an obscure problem in the future, if/when that UI feature is added.

EDIT: I see now that I cannot add this code because Staff::lines() does not take a parameter to indicate the measure, it only allows for one value for the staff for the entire score. So never mind.

On github, in the comments for this issue's pull request, I essentially opened a bug, which is the use of the raw literal "72" in the svggenerator.cpp file here:
https://github.com/musescore/MuseScore/blob/master/mscore/svggenerator…
It is a change I had claimed to have made as part of the pull request, but I failed to make it.
Should I open a new issue in order to fix it?

Re: SVG Export and DPI
-This new SVG export code exports without any scaling.
-72dpi is fixed internally in MuseScore, but the user display scales to a higher resolution.
-The result is an exported SVG that is absolutely fine, but that by default does not display on the screen at the same size as the score in MuseScore.

You might have noticed that the exported SVG appears smaller on your screen than the score in MuseScore. I can "fix" that, and the fix does not involve scaling all of the svg elements individually, as MuseScore did previously. It is a simple viewBox/size attribute manipulation, at the top of the file. If someone points me in the right direction for the 72dpi to actual screen resolution conversion factor, this is a simple change. I am already doing something similar, but fixed resolution, for my animated exports. I don't really have an opinion on the correct action here for MuseScore in general, and it doesn't affect my animated files, so let me know if this is a desirable change and where to look for a conversion factor. Thanks.

EDIT: As I continue working on related code I have this thought:
To implement staff space (spatium) scaling via the viewBox/size attributes.

I can see how for image capture copy/paste into any office or desktop publishing app this could be useful and even important. This way the user has consistency on their monitor across MuseScore and the external app, and the final non-MuseScore document contains items that are scaled the way the user intended.
-For now, no scaling at all is applied when creating the SVG.
-Previously, MuseScore SVG scaled based on the ratio between the default staff spacing (SPATIUM20) and the actual staff spacing, but it did so by scaling all the values in every element.
The viewBox/size attributes are the best place to do this ...if you want MuseScore to scale the SVG at all.
This would be in addition to the scaling for hi res monitors mentioned above. It would achieve maximum equivalence between the default size of the exported SVG and the size of the score displayed on the user's monitor within MuseScore.

I'm the one who wrote this code. This is certainly not a difficult change, but I think the real issue is that there are now two different requirements for SVG export, and the only way to resolve it is via a new user option of some sort.

That is if you're certain that the units being pixels is the source of the problem. Have you changed the viewBox, width, and height values to A4 numbers and applied the "mm" suffix in the svg file?

In my own version of MuseScore I have added a point/pixel option on the page layout dialog, so that points/pixels, which are currently the same in MuseScore at 72dpi, can be yet another base unit in MuseScore. Unfortunately that requires a change to the file format, which is not an easy thing to achieve in any opensource program. But adding that feature to MuseScore would pave a very logical path towards the viewBox, width, and height units for SVG files.

Otherwise there would need to be an option somewhere other than Page Layout to make this choice.

If you're serious about this you'll need to open a new issue, as this one is closed. If so, please include example file(s) and let's make sure that the issue is the pixel/point units, not something else. There are other sizing issues relating to screen size, but they shouldn't affect page dimensions.
If you are simply looking for a workaround you can obviously replace the pixel/point values in the viewBox, width, and height attribute values within the svg element itself, at the top of the file. I might be stating the obvious, but I want to make sure you have a way to function within the current release of MuseScore. You can use the page size values you know, be it A4 or letter or whatever, then follow that number with "mm". Or if you prefer you can inches, and then the suffix is "in". So viewBox="0 0 1920 1080" might become viewBox="0 0 35mm 25mm" (not that those numbers make any sense, but the formatting is correct). I hope that's clear or that you're already doing what I'm trying to communicate here.

It's also possible that there is a scaling factor that is distorting the numbers and it has nothing to do with the units being points/pixels or mm or inches. There is an outstanding scaling issue, and that's my concern with the way this is reported as a new issue.

Though if you're exporting the whole page, the Page Layout numbers in those same Page Layout units should cause the correct scaling regardless. If you're not exporting the whole page it's a different story.

There are various issues to consider in modifying the functionality, unfortunately. I would love to integrate points/pixels as Page Layout units in MuseScore, but my initial proposals about it were not greeted enthusiastically. As I hope I explained above, that would make this a straightforward upgrade to use the exact values/units used in the page layout.

And the viewBox should align with the width/height values, regardless of whether some programs ignore it. It's super simple to make align those values anyway.

Your welcome.

Regarding units, that's an interesting idea, putting the "pt" suffix after the number. But there are two reasons why that doesn't solve the problem:

1) points and pixels may be the same inside MuseScore since the 72dpi change, but that doesn't mean they're the same anywhere else.

2) there is still an outstanding scaling issue causing the pixel/point numbers to be incorrect.

I think the real solution is to use the numbers/units as specified in the Page Layout dialog whenever you export a full score.

Marc, the three issues you mention above are now fixed/merged (PR 2552). Although 105336 was not auto-marked as fixed because of my mistaken omission of the issue number in the commit message.

The issue that @musikai raises is still open, and still requires further discussion relative to a real solution. It does relate to the scaling issue(s) that is still open here:

https://musescore.org/en/node/100781

Though that is a "bug report", not an "issue".

I'm not sure if I'm available to work on this at this time. I can certainly assist you if you have any specific questions about the code. The staff lines code is designed to handle the most common cases with the greatest efficiency, but it should handle all the cases. In a situation where there are gaps in the staff lines, it processes them by measure, otherwise it processes them by system. I would think that the only change you would need to make is to enhance that if statement to check for staff type changes in addition to invisible staff lines.
As for the z-order, are you saying that by processing the staff lines separately this issue arises? If so, then it would probably be best to check for this extremely specific case and handle it as a special case. How often do z-order issues arise? This is the first I've heard of any.
The staff line code creates huge efficiencies in most SVG files, so please don't destroy it just to handle a few obscure cases. IMO it's important to handle the vast majority of cases cleanly, not to bloat all SVG files just because of a few obscure cases.

And what about 30 page scores for full orchestra? What about my horizontally scrolling scores that are one big system? What about people that want to edit their scores in Inkscape or a text editor (see previous comments in this issue)?

I had trouble fully understanding your post, but I have a basic sense of how to handle all this that I can summarize very briefly:
1) Change the if condition to include staff type changes as well as invisible staff lines
2) Do an initial scan for any elements that have a lower z-index than the staff lines (you can sort the elements in a page by z-order easily enough, AFAIK). Then "print" those to the SVG file prior to handling the staff lines. Then exclude them from later "printing".

Simple, right? I will take a look at the code in question sometime in the next few days and see if I can either make the change myself quickly enough, and if I can't I'll provide you with some detailed instructions. Can you give me a day or two to investigate this more thoroughly?

I'll build your PR on my Windows 10 machine and check it out. If your single-pass code works right, then kudos for consolidating stuff. I can't promise an exact date, but I should be able to get back to you within the week, if not sooner.