Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat svgz file as a vector image #22820

Merged
merged 2 commits into from
May 21, 2024

Conversation

mercuree
Copy link
Contributor

@mercuree mercuree commented May 14, 2024

Musescore suddenly supports compressed svg files (with .svgz extension) if you add them via drag&drop.
This PR fixes a situation where compressed svgs are incorrectly treated as raster images, which causes incorrect "pixelated" rendering and makes musescore slow.
Before:
before
After:
after

Explanations:
A compressed svg file is a regular gzipped svg.
Musescore accepts and renders svg files in these formats:

  • plain svg with .svg extension (correct rendering)
  • compressed svg with .svg extension (correct rendering)
  • compressed svg with .svgz extension (incorrect raster rendering, fixed by this PR)
  • compressed svg with .svg.gz extension (incorrect raster rendering, not covered by this PR)
  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 14, 2024

why not adding ".svg.gz" too?

@cbjeukendrup
Copy link
Contributor

Probably because the suffix will then be considered to be gz so withSuffix("svg.gz") doesn't work

@Jojo-Schmitz
Copy link
Contributor

Some code (from Mu3):

void MIconEngine::addFile(const QString &fileName, const QSize &, QIcon::Mode mode, QIcon::State state)
      {
      if (!fileName.isEmpty()) {
            QString abs = fileName;
            if (fileName.at(0) != QLatin1Char(':'))
                  abs = QFileInfo(fileName).absoluteFilePath();
            if (abs.endsWith(QLatin1String(".svg"), Qt::CaseInsensitive)
                || abs.endsWith(QLatin1String(".svgz"), Qt::CaseInsensitive)
                || abs.endsWith(QLatin1String(".svg.gz"), Qt::CaseInsensitive))
                  {
                  QSvgRenderer renderer(abs);
                  if (renderer.isValid()) {
                        d->stepSerialNum();
                        d->svgFiles.insert(d->hashKey(mode, state), abs);
                        }
                  }

@mercuree
Copy link
Contributor Author

mercuree commented May 14, 2024

why not adding ".svf.gz" too?

Initially I added support for .svg.gz, but it turned out that the file was still saved as .gz inside mscz. When you open the project, it will again be treated as a raster image. Additional changes need to be made, which is inconsistent with my almost zero C++ knowledge😁
Feel free to create more proper and complete PR and I will close mine

P.S.
MS3 has the same issue with drag&drop

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 14, 2024

P.S. MS3 has the same issue with drag&drop

Yes, I'm currently backporting this to Mu3. And found a few other places where we could/should support *.svgz, in file.cpp and in palette.cpp

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request May 14, 2024
Backport of musescore#22820

Plus some include files cleanup
@Jojo-Schmitz
Copy link
Contributor

in Mu4 it is in

std::vector<std::string> filter = { muse::trc("notation", "All Supported Files") + " (*.svg *.jpg *.jpeg *.png *.bmp *.tif *.tiff)",
muse::trc("notation", "Scalable Vector Graphics") + " (*.svg)",

,
{ "svg", ImageType::SVG },

and

@mercuree
Copy link
Contributor Author

Thank you, @Jojo-Schmitz! Added your changes. Everything seems to be working

@zacjansheski
Copy link
Contributor

Tested on MacOS 14, Windows 11, Ubuntu 22.04.3. Approved

@cbjeukendrup cbjeukendrup merged commit 449a64e into musescore:master May 21, 2024
11 checks passed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request May 21, 2024
Backport of musescore#22820

Plus some include files cleanup
@mercuree mercuree deleted the fix_svgz_display branch May 27, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants