regression with part resize

[spamatica]

I noticed it crashed when I tried to resize a part to the left recently.

I of course blamed myself due to having worked on this but the issue happened elsewhere.

This snippet from canvas.cpp
case DRAG_RESIZE:
if (curItem) {
if(resizeDirection == MusECore::ResizeDirection::RESIZE_TO_THE_LEFT) {
if (!supportsMultipleResize) {
QPoint rpos = QPoint(raster(pos).x(), curItem->y());
resizeToTheLeft(rpos);
curItem->move(start);
// Even though we only move the primary position here,
// set the mp as well.
//newCItem->setMp(newCItem->pos());
}

If I comment out that last newItem line it starts to work (since newItem is a null pointer).
I figured maybe it was supposed to be curItem but that behaves really wrong.
Has anyone touched this recently and know what it's supposed to do?
Judging by the comment it feels unimportant and my test works fine without it...

[terminator356]

Oops. My bad. Fixed regression at 23.05.2010
Late-night bleary-eyed copy/paste typo error.

Should work although untested. I think that section doesn't even have any bearing
on sounding notes while moving items anyway, but I was just being cautious and thorough.

Whenever an item's position changes, it is now important to change the mp (moving point)
in certain places as well, for the 'speaker' note-sounding logic, although we cannot simply
automatically do it inside move() so it has to be done place-by-place.

[spamatica]

No problem, I'm glad I'm not the only one introducing errors ;) ..oh well.
It's a complex thing to get right..

I saw your fix, and this was actually what I tried, for parts it does not behave right (but avoids the crash which is good). I get now that CItems can be notes too so I can have a closer look with that in mind, got no time right now though.

[spamatica]

Finally had a second look at this.
The line in question: curItem->setMp(curItem->pos())
fixes the crash but there is logic in the next step that wants the old mp...
As far as I can see, this code is only active for parts so maybe it is handled differently? I think it works when I remove this line.

[terminator356]

Hi. Looking at it now. Which "next step" do you mean?

[spamatica]

The PartCanvas::resizeItem(CItem* i, bool noSnap, bool ctrl)
it first checks that mp and pos are different and then goes ahead and uses the mp position to forward as newTickPos

[terminator356]

OK I removed it.
I'll deal with any further consequences requiring more fixes there.