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

Fix #303902: Export to SVG with trim via command line results in blank image #5970

Merged
merged 1 commit into from
Apr 28, 2020
Merged

Fix #303902: Export to SVG with trim via command line results in blank image #5970

merged 1 commit into from
Apr 28, 2020

Conversation

bsdz
Copy link
Contributor

@bsdz bsdz commented Apr 18, 2020

The removed translation moves the elements out of the viewport and repeats an identical translation 2 lines before.

Resolves: https://musescore.org/en/node/303902

This change simply removes what is probably a typo. The typo effectively doubled a translation that should only appear once.

I would like to add a test but unable to find test suite for comparing svg output with musicxml/mscx input. I have tested by hand and confirm the fix works.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

The addtional translation moves the elements out of the viewport.
@Spire42
Copy link
Contributor

Spire42 commented Apr 19, 2020

Please change the PR title to: “Fix #303902: Export to SVG with trim via command line results in blank image”.

You should probably also change the commit title to the same thing, make the commit message body more descriptive, and then squash.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Apr 19, 2020

That code got introduced in a9897a2, a long time ago, but for 3.x only it seems?
Being wrongly indented is another hint at a typo. @lasconic?

@bsdz bsdz changed the title Remove unnecessary painter translation. Fix #303902: Export to SVG with trim via command line results in blank image Apr 19, 2020
@bsdz
Copy link
Contributor Author

bsdz commented Apr 19, 2020

Please change the PR title to: “Fix #303902: Export to SVG with trim via command line results in blank image”.

You should probably also change the commit title to the same thing, make the commit message body more descriptive, and then squash.

I've changed the PR message. Won't change commit message as I feel it's sufficient.

@anatoly-os
Copy link
Contributor

@bsdz hello sir and welcome aboard! I don't understand the use case when you need trimming at all. Could you please explain the scenario when it is needed and what is the intended result?

@bsdz
Copy link
Contributor Author

bsdz commented Apr 21, 2020

@bsdz hello sir and welcome aboard! I don't understand the use case when you need trimming at all. Could you please explain the scenario when it is needed and what is the intended result?

Use case is identical to trimming PNG files which works fine. Typically you might want to export a cropped version of your score. See original issue which displays an example of trimmed PNG export.

@anatoly-os
Copy link
Contributor

@bsdz Do I understand correctly that you want to trim the blank space above the staff?

@bsdz
Copy link
Contributor Author

bsdz commented Apr 21, 2020

@bsdz Do I understand correctly that you want to trim the blank space above the staff?

Try the example in the issue with PNG.

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.

4 participants