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 #269870 SVG bugfix & unify page print function #3512

Closed

Conversation

ericfont
Copy link
Contributor

@ericfont ericfont commented Mar 3, 2018

printing a page follows same code strucutre regardless of SVG, PDF,
thumbnail, etc. This unifies the code between these print modes a
bit, and that helps out with keeping consistent behavior in the output.
Previously there was a bug with SVG output because it was trying to use
an optimization whereby staff lines were extended if all systems in a
measure were visible and weren't broken by frames. However, that
optimization had a mistake, whereby only the first measure of staff
lines was drawn. By unifying the print function, then also PDF can
benefit from the optimization. I've also improved the optimizatino a
bit to also handle all continuous measures even if the entire staff is
unbroken. I still need to investigate a bug with MMRests, though.

@ericfont
Copy link
Contributor Author

ericfont commented Mar 3, 2018

ok, as I said above, I am aware of a bug.

What I might do...I think I tried to do way too much in this... I think I should focus first on getting the SVG output correct. And then I could make another subsequent PR which tried to unify the all page print outputs.

Previously there was a bug with SVG output optimization which attempted to draw one set of staff lines for each system if there was no invisible staff lines or HBoxes, in which case it falled back to drawing staff lines measure-by-measure.  However, that optimization had a mistake, whereby only the first measure of staff lines was drawn for each system.  Also there would be segfaults in the middle of SVG output with MMRests.

This commit fixes that error and segfault.  It also improves the optimization by combining staff lines whenever there are contingous measures, even if only for part of a system.

It also unifies the page print function for all output modes (SVG, PDF, thumbnail, etc).  This helps out with keeping consistent behavior in the output, and also allows for PDF files to benefit from the optimization by having fewer elements.
@ericfont ericfont force-pushed the 822684-mergeSVG-Print-with-PDF branch from 3ce7cda to 69091f2 Compare March 4, 2018 06:02
@ericfont
Copy link
Contributor Author

ericfont commented Mar 4, 2018

I fixed the MM rest bug (which also happened to affect master). I've updated the commit message:

"Previously there was a bug with SVG output optimization which attempted to draw one set of staff lines for each system if there was no invisible staff lines or HBoxes, in which case it falled back to drawing staff lines measure-by-measure. However, that optimization had a mistake, whereby only the first measure of staff lines was drawn for each system. Also there would be segfaults in the middle of SVG output with MMRests.

This commit fixes that error and segfault. It also improves the optimization by combining staff lines whenever there are contingous measures, even if only for part of a system.

It also unifies the page print function for all output modes (SVG, PDF, thumbnail, etc). This helps out with keeping consistent behavior in the output, and also allows for PDF files to benefit from the optimization by having fewer elements."

I still should test more, but atleast my test file works also with MM rests enabled now.

@ericfont ericfont changed the title fix #822684 unify SVG & PDF & other page print (Work-in-Progress) fix #822684 SVG bugfix & unify page print function Mar 4, 2018
@ericfont
Copy link
Contributor Author

ericfont commented Mar 4, 2018

I should say, the PR looks like more changes than it really is. I could produce a smaller PR which only did the bug fix, without unifying, and that might help out for reviewing.

@ericfont
Copy link
Contributor Author

ericfont commented Mar 4, 2018

just a note to myself, travis failed because:

/home/travis/build/musescore/MuseScore/mtest/testutils.cpp: In member function ‘bool Ms::MTest::savePdf(Ms::MasterScore*, const QString&)’:
/home/travis/build/musescore/MuseScore/mtest/testutils.cpp:273:46: error: no matching function for call to ‘Ms::MasterScore::print(QPainter*, int&)’
cs->print(&p, n);

which is because I changed the functions around. I'll fix that in a couple days.

@lasconic lasconic added the work in progress not finished work or not addressed review label Mar 5, 2018
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Mar 7, 2018

Something is wrong about the commit message and PR title: there is no issue #822684. I guess you mean issue #269870, don't you?

@ericfont
Copy link
Contributor Author

ericfont commented Mar 7, 2018

You're right. I mistakenly looked at the number of a comment.

@anatoly-os anatoly-os changed the title fix #822684 SVG bugfix & unify page print function fix #269870 SVG bugfix & unify page print function Aug 7, 2018
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Aug 8, 2018

Can't this one get closed now, after #3853 ?
It at least needs a rebase

@anatoly-os
Copy link
Contributor

SVG has been fixed in c859eae.
Print functions unifying could be reapplied in the new PR.

@anatoly-os anatoly-os closed this Sep 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress not finished work or not addressed review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants