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 GH#145: Crash going from page to continuous view and then clicking on score element #151

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

Jojo-Schmitz
Copy link
Owner

@Jojo-Schmitz Jojo-Schmitz commented Aug 7, 2023

Resolves: #145
2nd attempt, after #146 failed, resp. caused #150

@worldwideweary
Copy link

worldwideweary commented Aug 7, 2023

Hm, this still seems to cause the problem the other guy mentioned with the footers not working right

It seems like the problem is that Mu4 has things like resetExplicitParent and text->moveToDummy(); ability which the MS3 codebase does not, but I might be wrong. Looks like that function some how makes the parent be attached to a "dummy" element founded in the score itself and not the page.... and Mu4 doesn't have the crash related to what i was getting in "3.7"... I'll see if I can figure something out there

Update: Nevermind, I had some changes remaining in the code while checking this. This should take care of both problems (i think).... since thetext->layout()function checks for a valid parent and sets offset to 0.0 if not, whereas setting parent to nullptr here after layout will bypass that problem in #150 all the while, so far, I'm not experiencing the crashes in continuous view as related in #145 so this seems good. Bunch of pretzel logic though. Feels like a re-vamp would help, but that isn't relevant. This makes the setting to nullptr in the drawing function redundant, it seems

Aside: never hurts to use nullptr and not the integer zero if you're not a "Vanilla C"-ist

@Jojo-Schmitz
Copy link
Owner Author

Hmm, I'm confused now (again), does this change fix your issue or is it no longer needed?

@worldwideweary
Copy link

Hey. This fixes the issue. But it looks like you merged #146 which means in a sense might need to do something else:

1

for instance needs to be reverted and i'm not sure what's the point of this:
2

If you were to revert that and then just apply #151... that would fix the original problem it seems to me. just that one setting parent to nullptr after the layout as this PR shows

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Aug 15, 2023

That converterMode to unrollRepeats change is entrirely unrelated (and fixes #143).
Reverting your (Page*)this to (0) change was needed to prevent #150.

The question is whether your issue #145 still exists and whether that one-line change of mine does any good?

@worldwideweary
Copy link

worldwideweary commented Aug 15, 2023

if all things were the same as they were at the moment of my posting the issue, and the only change from then was that there was:

text->setParent(nullptr);

after the layout as this PR has it, yes, it will fix the issue.

@Jojo-Schmitz Jojo-Schmitz merged commit 738ad58 into 3.x Aug 15, 2023
@worldwideweary
Copy link

So long as #146 is reverted, should be good to go.

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Aug 15, 2023

It is. This is merged now

@Jojo-Schmitz Jojo-Schmitz deleted the 3.x-crash branch November 24, 2023 16:59
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.

None yet

2 participants