MuseScore
MuseScore copied to clipboard
Fix #7819 - adjustment to previous page-clipping fix
Resolves: #7819
Had previously fixed this but code had been moved around since and additional logic around when to apply clipping added by others.
- [x] I signed the CLA
- [x] The title of the PR describes the problem it addresses
- [x] Each commit's message describes its purpose and effects, and references the issue it resolves
- [x] If changes are extensive, there is a sequence of easily reviewable commits
- [x] The code in the PR follows the coding rules
- [x] There are no unnecessary changes
- [x] 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)
I admit I didn't test during playback, can try, but I'm pretty sure the logical change is sound...
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Casper Jeukendrup @.> Sent: Monday, April 8, 2024 6:12:14 PM To: musescore/MuseScore @.> Cc: wizofaus @.>; Author @.> Subject: Re: [musescore/MuseScore] Fix #7819 - adjustment to previous page-clipping fix (PR #22135)
@cbjeukendrup commented on this pull request.
In src/engraving/rendering/dev/paint.cpphttps://github.com/musescore/MuseScore/pull/22135#discussion_r1555401453:
@@ -129,21 +131,14 @@ void Paint::paintScore(draw::Painter* painter, Score* score, const IScoreRendere }
// Draw page elements
-
bool disableClipping = false; -
if (!painter->hasClipping()) { -
painter->setClipping(true); -
painter->setClipRect(pageRect); -
disableClipping = true;
I think I've also seen that in master sometimes
— Reply to this email directly, view it on GitHubhttps://github.com/musescore/MuseScore/pull/22135#discussion_r1555401453, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI5UAJJ6AMO2BV6W2METJ3Y4JGN5AVCNFSM6AAAAABFSEZ7NOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSOBVG4ZDSNJQGQ. You are receiving this because you authored the thread.Message ID: @.***>
The vdiff failure here is real, it looks like anti-aliasing is not working properly. But the same is true for me when running a build from master! (actually I'd noticed that a bit recently, didn't think much of it)
Confirmed fix is still required & rebased.
I think this looks good now, @RomanPudashkin any objections?
However I'm still slightly disappointed that the initial fix of intersecting the existing clip rect with the page rect introduced a regression, because that was more elegant imo. I was wondering, would that regression be fixed by replacing https://github.com/musescore/MuseScore/blob/186c925cb4255e38b6f609868ad4049eff77c0c0/src/notation/view/abstractnotationpaintview.cpp#L1368-L1370 with
RectF dirtyRect1 = fromLogical(oldCursorRect).adjusted(-1, -1, 2, 2);
RectF dirtyRect2 = fromLogical(newCursorRect).adjusted(-1, -1, 2, 2);
or perhaps
RectF dirtyRect1 = fromLogical(oldCursorRect).adjusted(-2, -2, 2, 2);
RectF dirtyRect2 = fromLogical(newCursorRect).adjusted(-2, -2, 2, 2);
?
@RomanPudashkin which solution would you prefer, the current one or the original one?
As far as I can determine the original fix only didn't work reliably because of some bug in Qt, or something I couldn't figure out anyway. Artifacts still do appear from time to time even with the current master (but don't really cause a problem). But it really is only to handle a situation that shouldn't really happen under normal circumstances - in fact arguably MuseScore should never try to place elements that don't fit on the page (hence if you have too many staves, it should force introduce a mid-system page-break, or force scale everything down to fit).
@cbjeukendrup @wizofaus I've been playing with the playback cursor recently. My idea now is to draw it outside of the score view. Here's what it looks like: https://github.com/musescore/MuseScore/pull/24576/commits/af07099e87bb7ad31165d6af3be30b33ec33f8d0
This decently reduces CPU/GPU load during playback and we also don't have to increase the redraw area to fix this issue. Also, there will no longer be that problem when the playback cursor erases something during playback
@cbjeukendrup @wizofaus I've been playing with the playback cursor recently. My idea now is to draw it outside of the score view. Here's what it looks like: af07099
This decently reduces CPU/GPU load during playback and we also don't have to increase the redraw area to fix this issue. Also, there will no longer be that problem when the playback cursor erases something during playback
Is there a downside? (other than it using more QML, which I'm not really a fan of...) Is it worth me taking this commit and combining with my original clipping fix in this PR?
You don't have to do anything. Just wait until we merge my PR and rebase yours after that
@wizofaus please rebase
Suddenly I started doubting whether this is desirable. When things that fall off the page are not drawn, you won't ever notice that they are there:
- if an entire staff falls off the page, you can easily overlook that, and your full score won't have all instruments visible and the conductor will be confused
- if you have accidentally dragged an element off the page, and you can't see it anymore, you won't know that it is there. This means you're stuck with an obsolete element in your score; and more importantly, if that's an element that influences playback or something like that, you're going to have a hard time figuring out why playback is weird, because you have no way to see the element that's causing the weirdness.
Sorry for this very late realisation. But we'll need to give this some UX thought to this.
Of course, it is ugly, when stuff is drawn beyond the border of the page. But whenever that happens, it's a user error. If anything, we should try to prevent such user errors from happening; but we should not "mask" user errors, because that makes it impossible to correct them.
Definitely agreed - it's better to show the content off the page than not. But better still to figure out something reasonable to do. Merely scaling the page down to force it to fit isn't going to work well, because that will in turn impact how many measures fit on the page, which in turn may affect how much vertical space is required. Maybe a warning could be added during corruption checks?
It's supposed to be WYSIWYG, i.e. representing what your output will look like when you print it out. I don't believe there's any other such application out there that doesn't crop/clip items that don't fit on the on-screen representation of the page. And there may well be cases that's what the user wants, especially if it's just a design feature for a title page etc. Perhaps there's a case for having a mode where you can see items drawn off the edge of the page but what it does now by default very much looks like a bug - especially in the "publish" view.
I will note that the original bug reported was just "Instruments should not go outside the score if a lot of instruments added to the Score Wizard" - and hence fixing the clipping is not necessarily a sufficient solution, but I would argue it is a necessary one. I did actually previously propose that it should simply try to spread the system over multiple pages if it can't fit it on one (currently it does in fact move the system to the start of the next page if there are any text elements like "title" etc.).
But for other cases where symbols might end up off the edge of the page, then allowing them to be drawn outside the current page area is pretty obviously wrong, as they might even end up drawn over the previous or subsequent page area.
BTW if you do manage to modify an element so it ends up drawn onto another page (e.g. I did by changing the horizontal justification), you can't interact with it at all or determine what it belongs to:
Though interestingly with the keyboard you can even enter notes in staves that are off the bottom of the page:
Perhaps we could leave items drawn off the page in "Floating" view only, whatever that is... (I've never quite understood).
I can’t think of any graphics editors I have ever used that dont still draw elements when moved off the page. It’s actually a pretty crucial feature in that context. Dragging things off the page is less of a thing in MuseSvore, but it still seems way more natural to still see it than not. I’m more concerned with the case of a system extending past the bottom margin though, and making it as obvious as possible that this is what is happening.
Heh, I just tried using the "highlighter" Pen in MS Word Draw mode, and you do actually see it initially draw off the edge of the page boundaries, but then it cleans it up shortly after. Seems to be a quirk of that tool (Insert Shape is similar) as nothing else (WordArt etc.) ever draws off the edge of the page, and at any rate it's basically only while the mouse is captured and you're initially placing/sizing the elements. No other applications I tried (including Adobe Acrobat, etc.) allow drawing off the edge of a page at all, and elements that don't fit on the page are always clipped. Which examples were you thinking of? (and what did MS 3 do? Other notation software?)
BTW the current behaviour (in MS 4.3) when entering a line of text that won't fit on the page isn't great, you end up not being able to see what you're typing:
In fact in general items that extend over the right edge of a page are effectively clipped when the next page background is drawn anyway.
Google Drawings, Inkscape, etc. Drawing apps that allow you to place object on a canvas then move them. It’s a common technique to move objects off the page temporarily while working as a form of temporary storage for elements you aren’t sure you will want to use, like in doing A/B comparisons of different designs. Not that this relates directly to MuseScore, but the point is, it’s a very common pattern and the idea of having the off-canvas elements visible is something that I’d think most users would be quite familiar with. And as noted, it’s quite bad for elements you’ve moved off the page and potentially affecting layout and playback to be impossible to see and select.
Sure, I accept that there are some downsides to always clipping. But at very least it should definitely do so in Publish mode, and really we need to think a bit more about how to ensure that objects drawn completely outside the page boundaries can still be interacted with (because they can't with the mouse in the current version).
Clipping to page borders in Publish mode only seems a good compromise. And yes, it is indeed problematic that items outside a page (or dragged to the wrong page, even) cannot be clicked. Probably not trivial to solve. But anyway, being able to see the element at least is still better than not being able to see it anymore at all.
That should be relatively easy to change, but it's worth asking then if it really resolves the bug as reported.
Is there any case for having systems split over multiple pages if required? (I have some study scores that do that, though they flip into landscape mode, and it's only ever across two "open" pages, never over a page turn).
Seems pretty clear the person reporting wouldn't be happy with any proposed rearranging of the pixels that end up on the outskirts of the page. They presumably just want the music to actually fit on the page. Which isn't a bad thing to want, but as noted, that a much harder problem.
Anyhow, I agree, not showing the off-page elements in publish mode seems like a no-brainer. Potentially could also disappear if you turn off display of invisible elements, even though that of course isn't really right.
Tested on MacOS 14, Windows 11, Ubuntu 22.04.3. Approved