Skip to content

Fix element stacking during partial layout#32755

Merged
mike-spa merged 1 commit intomusescore:masterfrom
CubikingChill:fix-element-stacking
Mar 30, 2026
Merged

Fix element stacking during partial layout#32755
mike-spa merged 1 commit intomusescore:masterfrom
CubikingChill:fix-element-stacking

Conversation

@CubikingChill
Copy link
Copy Markdown
Contributor

@CubikingChill CubikingChill commented Mar 24, 2026

Elements (text, dynamics) were stacking on top of each other after edits, only returning to correct positions after save/reload. This occurred because partial layout reset only affected the edited measure range while skyline collision detection processed entire systems, creating mixed stale/fresh data.

Changes:

  • Enable position reset in LayoutData::reset() to force clean recalculation
  • Reset autoplace state to ensure proper collision detection
  • Expand PassResetLayoutData to reset all measures in affected systems, not just the edited range

Fixes the mismatch between partial reset scope and full system processing.

Resolves: #32756

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@shoogle shoogle marked this pull request as draft March 24, 2026 12:40
@shoogle
Copy link
Copy Markdown
Contributor

shoogle commented Mar 24, 2026

Please keep PRs as draft until all checks have passed.

  • Unit tests: Search the log for @@ to find diffs where your code produced different output to the reference files in the repository. Once you've fixed those, if it still fails, search for fail and error to fix other kinds of failures (but these will return some false positives like non-fatal main_thread errors not related to the tests).
  • Visual tests: Download the Comparison artifact from the Summary page inspect the files inside.

Also see the wiki page: Fix the CI checks.

@avvvvve avvvvve requested a review from mike-spa March 24, 2026 15:06
@CubikingChill
Copy link
Copy Markdown
Contributor Author

Please keep PRs as draft until all checks have passed.

* **Unit tests:** Search the [log](https://github.com/musescore/MuseScore/actions/runs/23485663815/job/68340449428?pr=32755) for `@@` to find diffs where your code produced different output to the reference files in the repository. Once you've fixed those, if it still fails, search for `fail` and `error` to fix other kinds of failures (but these will return some false positives like non-fatal `main_thread` errors not related to the tests).

* **Visual tests:** Download the Comparison artifact from the [Summary page](https://github.com/musescore/MuseScore/actions/runs/23485663796) inspect the files inside.

Also see the wiki page: Fix the CI checks.

Thanks for reminding.

@CubikingChill CubikingChill marked this pull request as ready for review March 24, 2026 17:57
Copy link
Copy Markdown
Contributor

@mike-spa mike-spa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CubikingChill I see no problem with your code but I can't understand the issue that you are trying to solve as it's impossible to understand it from your description. Can you please attach screenshots to your issue, or even better a video capture? Thank you!

@CubikingChill
Copy link
Copy Markdown
Contributor Author

Sure. Screen recording on the way.

@CubikingChill
Copy link
Copy Markdown
Contributor Author

2026-03-27.15-15-06.-.Trim.mp4

Elements (text, dynamics) were stacking after edits because autoplace
collision detection used stale offset state. Clear autoplace state
(offsetChanged, changedPos) during layout reset to force fresh collision
detection without resetting positions.
@mike-spa mike-spa merged commit 138844a into musescore:master Mar 30, 2026
12 checks passed
@davidstephengrant
Copy link
Copy Markdown
Contributor

I cannot see that the linked issue has been fixed in this PR.

Here in a fresh master build:

Screencast.from.2026-03-30.12-37-36.webm

@CubikingChill
Copy link
Copy Markdown
Contributor Author

2026-03-30.12-51-45.mp4

Interesting. The PR build is correct tho.
https://github.com/musescore/MuseScore/actions/runs/23655886275

@davidstephengrant
Copy link
Copy Markdown
Contributor

The bug only presents in continuous view. In the recording above you are in page view.

Screencast.from.2026-03-30.14-01-09.webm

@CubikingChill
Copy link
Copy Markdown
Contributor Author

CubikingChill commented Mar 30, 2026

You are right. Sorry for my careless observation. I will work on that again.

@mike-spa
Copy link
Copy Markdown
Contributor

@CubikingChill no, you will not.

We have noticed that in all of your PRs, the requirement that The code compiles and runs on my machine is not checked. That means you are literally contributing AI slop without even checking first that your code works and does what you want. Additionally, the fact that you post more AI slop as a "summary" of your code changes makes me highly suspicious that you don't even understand the code you are contributing, because if you did you could have summarized that yourself in a few words.

It is one thing to use AI to speed up the work. It is a different thing to blindly send us AI code that you haven't even compiled. It's disrespectful to us, as we all need to waste our time checking and testing and understanding dozens of low quality contributions that you haven't checked and tested and understood yourself. And it's disrespectful to all other community members who've contributed their own genuine work and time and skills over the years.

From now on, I will close any PR where The code compiles and runs on my machine is not checked. You are free to keep contributing, as long as

  1. You compile your code locally, and verify that the code does what you want before opening a PR in our repository.
  2. You demonstrate genuine understanding of the code you are contributing by properly answering the maintainers' questions, something which you have repeatedly failed to do Fix crash on creating a score by adding null check and fallback for iocContext #32801 Optimize lyrics underscore input performance #32835 Fix note input disabled after sleep/wake #32836 Reduce lag when opening parts for the first time. #32837
  3. You provide at least one screen capture in each PR showing precisely what your changes have achieved. If your changes are about performance improvement, such as Optimize lyrics underscore input performance #32835, you should test them using the TRACEFUNC macro and report the differences in run times.

@CubikingChill
Copy link
Copy Markdown
Contributor Author

CubikingChill commented Mar 30, 2026

Hello. I do admit that I didn't setup the environment on my PC. But for most PRs I did test at least the Linux x86 from the workflow.

If testing the workflow build counts as "The code compiles". I am happy to do so in future. And I am also very willing to apologise for any carelessness when I test my build.

If you need extra guarantee, I don't mind submitting a short video showing that the build works and makes a difference

Last but not least, I admit that some of the PRs I created are of little effort. I promise that from now on I will put more efforts before marking PRs as read for review.

@mike-spa
Copy link
Copy Markdown
Contributor

If testing the workflow build counts as "The code compiles". I am happy to do so in future.

No, it doesn't. Automated builds are meant to be used as an integration check and provide a way for non-developers to test the changes, they are not developer tools. If you want to contribute code, you set up your environment, build the software, make your changes, see if they work, and then you contribute. Thanks.

mike-spa added a commit to mike-spa/MuseScore that referenced this pull request Mar 30, 2026
…ment-stacking"

This reverts commit 138844a, reversing
changes made to 5247ebc.
@CubikingChill
Copy link
Copy Markdown
Contributor Author

t to be used as an integration check and provide a way for non-developers to test the changes, they are not developer tools. If you want to contribute code, you set up your environment, build the software, make your changes, see if they work, and th

Noted.

mike-spa added a commit that referenced this pull request Mar 30, 2026
Revert "Merge pull request #32755 from CubikingChill/fix-element-stac…
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.

Element Stacking Bug During Partial Layout

4 participants