GSoC 2020: Tree Model - Week 4 (scanElements refactoring)
Hello there! This is the latest blog in my series of GSoC blog posts.
Last week, I had planned that I will be working on refactoring the scanElements function using the score tree functions. I was able to nearly complete it but just near the end of the week, I found an interesting bug, because of which I'll have to make some changes either to the tree model or some code in Page or Measure classes. So it'll probably take a little longer to finish this refactoring.
You can see the latest changes I'm making on this PR on GitHub: https://github.com/musescore/MuseScore/pull/6274
So here's a summary of what I did:
I renamed the scanElements function to
scanElementsOldand created a new
scanElementsfunction in its place.
scanElementswas decalred as a virtual function in the
Elementclass but it was also implemented in
Scoreclass without overriding the virtual function. So apparantly it should have been declared in the
ScoreElementclass from the start? Maybe it wasn't done that way for historical reasons. Anyways I created the new scanElements in the ScoreElement class.
I created a new test that will compare the elements scanned by
scanElementsOld. I created this test, so that when this test passes I will know that I have refactored scanElements correctly and it will not change any exisiting functionality. I will remove this test later, along with the old scanElements function once everything seems to work.
I implemented the new scanElements function using a tree traversal. This is what the basic implementation looks like:
This function needed to be overriden in a small number of classes. For example in class
Restwe need to check the
isGap()function, and also call the given function on
thisin the above mentioned classes, etc.
After some testing, It turned out that I had missed two elements, Tuplets and Melisma lines in my original tree model, which I added in another commit..
After a while I was able to make sure that scanElements is working same as before on the 4 scores I used for testing (two artificial ones, one was a copy of the Moonlight Sonata, and one was the Goldberg variations by Bach).
After this I tested the application with the new scanElements and it seemed to be working completely okay, and I could find no obvious bugs yet.
Now we come to the interesting part... One test (
tst_unrollrepeats) was still failing on travis. I tested it locally and tried to find what was going wrong. I thought that it must be some trivial bug and this PR would also be finished soon. But it didn't turn out to be the case ;)
It seemed like old and new scanElements were scanning different elements after repeats were unrolled in the test score. However when I checked the generated score with
tst_scanElementsI didn't get any difference. Then after some debugging I found out the reason for it. After unrolling the repeats, new measures were added to the score, but it seems like the Page and System classes aren't getting the updated list of measures until a layout is done.
The original scanElements scans Measures, Pages and Systems separately, like in the left diagram (the measures are in a doubly-linked list, and the newly inserted measure is dashed), whereas the new one follows the tree structure like in the new one. So the original scanElements is able to see the newly inserted measures but the new implementation doesn't see those unless I call score->doLayout() first.
So I have a few ideas in my mind to fix this problem:
- Use the same logic as before in scanElements in the Score class.
- Change the score tree to match the scanElements function.
- Modify some code in Measure class so that as soon as a measure is constructed it is added to some page or system (possibly the same system as previous one), but the layout is not done yet.
- Call doLayout() after every Measure insert?
I'll try to test some of these options this week and I'll try to figure out what's the best way to fix this problem.
PR on GitHub: https://github.com/musescore/MuseScore/pull/6274
Previous Blog: https://musescore.org/en/user/1743616/blog/2020/06/22/gsoc-2020-tree-mo…
Next Blog: https://musescore.org/en/user/1743616/blog/2020/07/06/gsoc-2020-tree-mo…