Adding articulation causes full relayout

• May 31, 2019 - 22:54
Reported version
3.1
Priority
P3 - Low
Type
Functional
Frequency
Once
Severity
S3 - Major
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project
MuseScore

See https://musescore.org/en/node/290027 for a sample score and the initial report.

Adding articulations causes a full relayout and makes it seem very slow.

Comments

• May 31, 2019 - 23:58

I thought this would be a very simple fix, and it may yet be, but not as simple as I thought. There is a triggerLayout() call in Element::setProperty() that is causing the full layout because this is happening before the element has a parent and thus before we can determine its tick. It turns out this happens in the Articulation constructor, so we don't really have an opportunity to set the parent. So, maybe we can get away with only doing the triggerLayout() is the element has a parent. Not sure if this ends up causing something to be missed, but I doubt it.

For the record, the specific property being set that causes this is MIN_DISTANCE, so it was related to my skyline changes. Still, it's a more general problem than that, I can see other elements cause the same on being added.

Reply

• Jun 1, 2019 - 22:42

Indeed, many if not most elements seem to be triggering a full layout when added, for no good reason except that Element::setProperty calls triggerLayout(), which returns 0 if the element if has no parent (and this is the norm when first adding an element). So actually, it's not a full relayout necessarily, but from the beginning of the score to the point you are adding, meaning adds get slower the farther in the score you add. Eg, a score of 200 measures, adding to measure 5 is fast, adding to measure 190 not so much. Also, it's not just MIN_DISTANCE for other elements, so this would have been the case in 3.0.5 as well, although for articulations specifically it's probably new for 3.1.

So far, having triggerLayout() do nothing if the element has no parent seems like a winner. I haven't made it break yet, and even if something does break, that tells me there was a missing triggerLayout() call elsewhere (or we fail to set parent for some element at all) and we should fix it there rather than relying on this.

Reply

Fix version
3.2.0