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 #308568: bad selection and corruption on delete #6415

Merged
merged 1 commit into from
Sep 1, 2020

Conversation

MarcSabatella
Copy link
Contributor

@MarcSabatella MarcSabatella commented Aug 4, 2020

Resolves: https://musescore.org/en/node/308568

If you make a range selection "in reverse",
by selecting a note then extending it to the left
(whether using Shift+Left or Shift+click),
you get a bad selection where the endSemgent is in the next measure.
And in the case of a measure at the end of a system,
it means the selection includes the header of the next system.
This glitch has been the case since MuseScore 2 at least,
but with the reimplementation of measure delete as time delete,
it actually causes corruption when deleting the select range.
That is because we are getting the wrong measure
for the end of the selection, and attempting to do a partial time delete
(with length of 0) in the next measure.

This change fixes the basic problem with selection, so reverse selection
no longer causes this bad result.
This is done by adjusting the initial range to end with
the first segment after the originally-selected CR
rather than the last.
I also fix the calculation of the range of measures
to not be fooled if the endSegment comes after a header -
this was causing an unnecessary invocation of time delete
to delete the beginning of that next measure.
Finally, I also check to be sure we aren't trying to delete zero time,
since that was where the corruption was coming from.

@MarcSabatella
Copy link
Contributor Author

Not sure what that failure is about.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Aug 26, 2020

importexport.lib(importmidi_chord.obj) : error LNK2019: unresolved external symbol "__declspec(dllimport) public: __cdecl 
QLinkedListData::QLinkedListData(void)" (__imp_??0QLinkedListData@@QEAA@XZ) referenced in function "private: class 
QLinkedList<class Ms::MidiNote>::iterator __cdecl QLinkedList<class Ms::MidiNote>::detach_helper2(class QLinkedList<class 
Ms::MidiNote>::iterator)" (?detach_helper2@?$QLinkedList@VMidiNote@Ms@@@@AEAA?AViterator@1@V21@@Z) 
[C:\MuseScore\msvc.build_x64\main\mscore.vcxproj]
C:\MuseScore\msvc.build_x64\main\RelWithDebInfo\MuseScore3.exe : fatal error LNK1120: 1 unresolved externals 

You haven't touch any code in the vicinity, so I don't know why this fails?

Maybe just try to build it again? Might have been the Qt/MSVC Bug we fixed by updating to a patch up Qt 5.9.9. Actually I'm quite sure it is.

Resolves: https://musescore.org/en/node/308568

If you make a range selection "in reverse",
by selecting a note then extending it to the left
(whether using Shift+Left or Shift+click),
you get a bad selection where the endSemgent is in the next measure.
And in the case of a measure at the end of a system,
it means the selection includes the header of the next system.
This glitch has been the case since MuseScore 2 at least,
but with the reimplementation of measure delete as time delete,
it actually causes corruption when deleting the select range.
That is because we are getting the wrong measure
for the end of the selection, and attempting to do a partial time delete
(with length of 0) in the next measure.

This change fixes the basic problem with selection, so reverse selection
no longer causes this bad result.
This is done by adjusting the initial range to end with
the *first* segment after the originally-selected CR
rather than the last.
I also fix the calculation of the range of measures
to not be fooled if the endSegment comes after a header -
this was causing an unnecessary invocation of time delete
to delete the beginning of that next measure.
Finally, I also check to be sure we aren't trying to delete zero time,
since that was where the corruption was coming from.
@Jojo-Schmitz
Copy link
Contributor

OK, fine this time ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants