Re: Undo Toolbar Button Refresh Issue

Ted Felix <ted-Ll1PANez7xxWk0Htik3J/[email protected]> Thu, 6 Feb 2025 13:30:48 -0500
Newsgroups gmane.comp.audio.rosegarden.user
Message-ID <[email protected]>
   Just pushed a new fix [de2d72f2] for an old problem.  Using the undo 
*toolbar button* in the matrix sometimes didn't refresh.  Especially 
with the playback pointer in the segment.

   Please test latest git.  Here's the procedure:

- Create a big segment.  8 bars or so.

- Open the matrix on that segment.

- Make sure the playback position pointer is in the segment.

- Add 30 or so random notes so that you can see them all at once.

- Use the *toolbar* undo button (not the toolbar undo history drop down,
not Ctrl+Z and not the menu) to remove the notes one by one, paying
attention to whether they go away each time.

- Confirm that each note goes away each time the undo toolbar button is
pressed.

   This is an ancient patch from David Faure from 2022 that fell through 
the cracks.

   Complete details are in this thread on the devel mailing list:

https://sourceforge.net/p/rosegarden/mailman/rosegarden-devel/thread/2982dd28-4a1e-3f41-c94b-11e231c31969-Ll1PANez7xxWk0Htik3J/[email protected]/

Ted.

On 6/6/22 9:47 AM, David Faure wrote:
> Debugging this a little bit further (I like tricky bugs) shows a number of
> surprising facts.
> 
> 1) One undo step sometimes calls Segment::notifyRemove more than once.
> 
> This is because BasicCommand::unexecute() calls copyFrom() which does
> 
> m_segment->erase(m_segment->findTime(m_modifiedEventsStart),
>     m_segment->findTime(m_modifiedEventsEnd));
> 
> As the comment in BasicCommand.cpp:302 says, surely this copyFrom() should be
> done without firing notifications?
> 
> Note however that this isn't the actual problem. It's just rather confusing
> while debugging ;)
> But even if MatrixScene::handleEventRemoved() is called multiple times,
> the update() in there has no effect after the first call.
> The first call sets d->updateAll to true (in QGraphicsScene), the following
> calls return immediately because updateAll is true. It gets set to false
> whenever actual drawing happens (QGraphicsScene::drawItems, called by
> QGraphicsView::paintEvent).
> 
> 2) While debugging deep within Qt, I found out that the "dirty" region of the
> graphicsview's viewport is only 4 pixels wide, because
> Panned::showPositionPointer calls viewport()->update(oldRect) which is a 4
> pixels wide rectangle. Normally the graphicsview is repainted via the scene's
> update() mechanism (which stores a dirtyRegion in QGraphicsView itself). I
> think the heart of the bug is a collision between the two update mechanisms.
> The scene thinks it's going to trigger a repaint of the whole view, but the
> view has stored only a tiny update region.
> 
> I think it can be fixed by using scene()->update() instead.
> Patch attached, please test (I have of course reverted the commit that removes
> the update() call from MatrixScene::handleEventRemoved).
> 
> But this makes me wonder, why is the "pointer" drawn in drawForeground, after
> these complex manual updates, instead of making the pointer a graphics item
> and letting QGraphicsScene/QGraphicsView take care of updating correctly for
> that item?
> 
> 
> 
> _______________________________________________
> Rosegarden-devel mailing list
> Rosegarden-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org - use the link below to unsubscribe
> https://lists.sourceforge.net/lists/listinfo/rosegarden-devel