Re: Undo Toolbar Button Refresh Issue

chuck elliot <[email protected]> Fri, 7 Feb 2025 11:22:49 +0000
Newsgroups gmane.comp.audio.rosegarden.user
Message-ID <[email protected]>
'tried that - all good 🙂


On 06/02/2025 18:30, Ted Felix wrote:
> 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/[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
>> [email protected] - use the link below to 
>> unsubscribe
>> https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
>
>
>
> _______________________________________________
> Rosegarden-user mailing list
> [email protected] - use the link below to unsubscribe
> https://lists.sourceforge.net/lists/listinfo/rosegarden-user


_______________________________________________
Rosegarden-user mailing list
[email protected] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-user