allow insert Measure, VBox, or HBox before a VBox or HBox

• Sep 22, 2015 - 04:37
Type
Functional
Severity
S4 - Minor
Status
active
Regression
No
Workaround
No
Project
MuseScore

My feature request (https://musescore.org/en/node/78341) was approved, so I'm copying here into issue tracker:

...Musescore won't let me insert a Measure, VBox, or HBox before a VBox or HBox.
Selecting a V/H Box and pressing insert button on keyboard or double clicking a V/H Box or Measure from palette, or dragging & dropping a V/H Box or Measure from palette onto the V/H Box...all result in the V/H Box or Measure being appended to the end of my score.
But I want the V/H Boxes and Measures to be inserted before the V/H Box.

Comments

• Nov 28, 2015 - 17:11

Eric - were you working on this? I was about to make another change to a part of the code that is involved here and stumbled on this in my testing. I think I've found the cause, and it's pretty simple:

https://github.com/musescore/MuseScore/blob/7321ba6588039f5fa34036e05d4…

We are trying to find the MeasureBase to insert in front of, and failing unless there happens to be a linked part already. The fix is, I believe, just a matter of returning mb in this case (maybe checking first to see if mb->score() matches score).

Also, I'm changing from feature request to bug report because the current behavior is clearly incorrect. Not only is it unexpected, but bad things happen if there are spanners attached to any measures after the insertion point. Their start & end points become incorrect, as the call to undoInsertTime() within Score::insertMeasure() is actually trying to do the right thing and insert the time *before* the selected frame, even though the measure itself is added to the end. probably there are other bad things that happen as a result too.

Reply

• Nov 28, 2015 - 17:35

Sometimes I also want to insert a frame before another frame, that doesn't work as you need to have a measure selected, maybe this issue gives the chance to fix this too?

Oops, just see that this is covered in the issue, so here's my 'metoo' ;-)

Reply

• Nov 28, 2015 - 19:09

A horizontal before a vertical should result in some measure, the horizontal frame, a(n implicit) line break and a vertical frame.
If you have a score that has a vertical frame already and figur that the last measure before that needs a horizontzal one, this currently requires you to hop thoohg a couple loops: remove the vertical frame including its content, insert the horizontal frame, then the vertical one, then restore the frame's Content.
Same story with adding a vertical frame before a horizontal one, not sure your PR fixes that?

Reply

• Nov 28, 2015 - 19:22

Inserting a *measure* is very different from inserting a *frame*. My fix is for measure insert, not frame insert. It just happens to imprve the frame insert case in that things will no longer be appended in the cases where the operation was supported in the first place. But it remains the case we don't actually seem to actually *support* horizontal frames in front of other frames. The use case you propose does seem worth considering, but I also suspect many would expect the horizontal frame to simply be inserted on the same "system" as the vertical frame, thus pushing the vertical frame to the right. Results with my PR seem indeterminate, but layout is not good in some case, suggesting we just don't support this at a deeper level.

Reply

• Nov 28, 2015 - 19:32

Sorry guys I've been away from coding for a couple months...I should have unassigned myself, so you can assume anything I assigned myself to is unassigned, until I start coding hopefully in a couple weeks.

Reply

• Nov 28, 2015 - 19:35

OK, I've just tested: inserting any frame in front of a horizontal one doesn't work, 'you need to have a measure selected' pops up. Inserting measures in front of frames also doesn't work, same reason.
Inserting a horizontal Frame in front of a vertical one works.

Reply

• Nov 28, 2015 - 19:38

The fix seems rather simple: in libmscore/element.cpp change line 1619
from
if (type() == Element::Type::MEASURE)
to
if (type() == Element::Type::MEASURE || type() == Element::Type::HBOX)

Why VBOX doesn't need to get added there too is beyond me though.

Reply

• Nov 28, 2015 - 20:08

Note that handling of the Add menu is different than handling of the palette, as I alluded to earlier. The dialog you mention only appears when using the menu. When using the palette, the operation either succeeds or fails silently, and it succeeds in some cases where the menu doesn't. But some of those "successes" do not lay out correctly.

EDIT: also, I don't think simply changing findMeasure() to allow it to return something other than a measure is safe. The return value is then cast to a Measure, and if it *not* in fact a measure, all sorts of random bad things could happen. We need to change the cast to MeasureBase, or refactor some of this code a bit.

Reply

• Nov 28, 2015 - 20:19

The reason we wouldn't need to handle VBOX in findMeasure() is that those are handled upstream in ScoreView::checkSelectionStateForInsertMeasure(), right before the error message is displayed. So really, *that's* where we'd need to add the test for HBOX to get the add menu to succeed, but still, as I mentioned, the operation doesn't *really* work in the case of inserting a horizontal frame before a vertical one - layout is messed up unelss there is a line break on the horizontal frame. Actually, though, that too is probably a relatively simple fix involving the code where we are cllecting measures into systems, so I will look into that and hopefully have a more complete fix for this issue soon.

Reply

• Nov 28, 2015 - 20:30

See also #65401: Inserting / appending horizontal then vertical frame causes system to split. There is a basic layout problem with horizontal frame followed immediately by vertical one. I updated to allow the insert of the verttical frame in front of the horizontal one, but as mentioned, layout is wrong. You see similar results without my PR if you insert the frames in the opposite order: first insert the horizontal frame, then with the next measure still selected, insert a vertcial one.

So I'd consider my PR here sufficient to close this issue (once merged), but we need to look at the layout issue as well at some point.

Reply

• Nov 28, 2015 - 20:34

OK, that's a better place indeed and does the trick too. And yes, a line break needs to get added too, somehow, explicit or, better, implicit (the same way as if they get added the other way round)

Reply

• Mar 18, 2016 - 00:59

I'm noticing in that situation, Score::insertMeasure(Element::Type type, MeasureBase* measure, bool createEmptyMeasures) will get called with measure=NULL.

Edit: and in that case, the HBox will incorrectly get appended to end of score.

I'm also noticing that if I select the HBox/VBox in the score that I want to insert before, and then double click the HBox from pallete, then nothing happens. I can drag the HBox from pallete to my desired insert point, and then it gets inserted to end of measure, since apparently Score::insertMeasure gets called with NULL measure. So I'm thinking this error is not in Score::insertMeasure, but rather in some other logic that searches for the insert point.

Reply

• Mar 18, 2016 - 01:17

noticing up the stack trace, that ScoreView::dropCanvas is what calls Score::insertMeasure, which uses NULL for the measure object to insert before. So the VBox is somehow not even being selected as insert point...

Reply

• Mar 18, 2016 - 01:35

in ScoreView::dropEvent(), if want to drop HBox, then the lines

for (const Element* e : elementsAt(pos)) {
                              if (e->acceptDrop(dropData)) {
                                    el = const_cast(e);
                                    break;
                                    }
                              }

don't ever acceptDrop.

Edit: So I'm thinking that Box::acceptDrop needs to accept drops form HFRAMES.

Reply

• Mar 18, 2016 - 01:52

It is conceivable there is some reason why HFRAME is not included in Box::acceptDrop(). Maybe it has something to do with wanting to be able to add an HFRAME *within* a VFRAME (not that this change would break that), or maybe it has to do with the notion that two HFRAME's in a row doesn't make sense (even though it actually does, if one ends a system and the other starts the next system). But anyhow, your analysis makes sense to me; if adding that one line fixes it, I'm happy.

Reply

• Mar 18, 2016 - 01:57

thanks for replying marc. I see nothing wrong with the notion of dropping a HFRAME before a VFRAME. Of course inserting inside of a VFRAME doesn't make sense...but it seems the semantics of all the measurebase object drops specficially is interpretered as insert *BEFORE* the object we're dropping onto...so I think this supplementary fix is consistent with that interpretation.

Reply

• Mar 18, 2016 - 02:02

well it seems I found a case where inserting HFRAME before VFRAME has produced bogus behavior. If start with:

want-to-insert-hbox-before-this-vframe.png

and then select that VFRAME, and (with my commit applied) if I drop HFRAME, then:

after-insert-hbox-before-that-vframe.png

Reply

• Mar 18, 2016 - 02:09

another instance of bogus behavior, if I want to insert vframe before this final measure:

want-to-insert-vbox-before-final-meas.png

then the result has no *visual* change:

after-insert-vbox-before-final-meas-no-visual-change-happens.png

but if I insert another vframe before that final measure, then finally I see 2 vframes:

after-insert-vbox-before-final-meas-twice-then-both-finally-appear.png

Reply

• Mar 18, 2016 - 09:13

I've verified on latest nightly, that layout issue was already existing, so I've made a seperate issue report: #102576: VBox immediately after HBox in pattern of Measure(s),HBox,VBox,Measure(s) does not split system. It also has nothing to do with #65401: Inserting / appending horizontal then vertical frame causes system to split.

I've fixed both problems and am submitting PR... https://github.com/musescore/MuseScore/compare/master...ericfont:78636-…

Reply