OpenBoard-org / OpenBoard

OpenBoard is a cross-platform interactive whiteboard application intended for use in a classroom setting.
https://openboard.ch/
GNU General Public License v3.0
2.26k stars 413 forks source link

Artifacts when moving/rotating objects #827

Open Vekhir opened 8 months ago

Vekhir commented 8 months ago

Issue

When moving or rotating an object "quickly", OpenBoard leaves artifacts visible. This is particularly prominent when zooming in, e.g. rotating the ruler at zoom level 2.1x leaves artifacts after almost any movement, making it really easy to test. I was able to reproduce the issue with other widgets and self-made drawings. Lower zoom level (e.g. 1.1x) usually means faster movement to create artifacts. Zoom levels at or below 1 don't seem to be affected in Qt5, but are in Qt6.

More information

The issue occurs both with Qt5 and Qt6, relevant versions below. Interestingly, if I start a recording, then the artifacts stop occurring. The already existing ones stay until some other object comes near them, but no new artifacts are produced. This is true even after the recording is stopped - even if it is only a second long - and the podcast feature is disabled again (clicking "Openboard" > "Podcast" again). Was it just a moment ago basically impossible not to leave artifacts, it's now incredibly difficult to produce them. I think I still got one small artifact afterwards, but they are essentially gone. Restarting OpenBoard makes the artifacts occur again.

Is this something you are aware of, especially the behaviour with the podcast feature?

Bisection

This issue occurs on 1420d5f36cb9188081b6b9bccaeedcb1f0e97f8e (current master) This issue occurs on 4f49a2fa0e8feb626ca20957c9c9e2296dedf93a This issue occurs on 374d5b4d1aa94e5e4c79120eec93a9141456a89a (introduces startup hints) This issue occurs on 3de1e20fa038ddd69dc12b7d0bebac181ac4323e This issue occurs on acbfc7507db8770f8707685a5bcde9863736c37c This issue occurs on a7c16ca1f516ff04cde243127316e06a5f86cfc7 This issue occurs on 35d4356f96dfff86142fe94a0af433dea21cb4f3 This issue occurs on a504dbee0972353631bfc4f9b2e94f67aa4ad28c (at > 3x) This issue does not occur on 2f4f94aa965611936e6882fa30ecea3becefee48 This issue does not occur on 6119753b7f8055620ec506522744a7f10b2615e8 (first dev merge into master) This issue does not occur on the released package 1.6.4.

System information

OS: Arch Linux Kernel: 6.6.1.arch1-1 openboard-git: see above section "Bisection" Qt5: 5.15.11+kde+r146-1 Qt6: 6.6.0-3 ffmpeg: 2:6.0-13

Vekhir commented 8 months ago

Based on my initial investigation above, the commit a504dbee0972353631bfc4f9b2e94f67aa4ad28c is at least partially responsible for the observed behaviour. The commit immediately preceding it (2f4f94aa965611936e6882fa30ecea3becefee48) does not show any artifacts even at 9.0x zoom level, while the offending commit above shows artifacts for any zoom level above 3x. For zooming levels below 3x, the commit does not show artifacts. Also, the podcast fix works here already.

Vekhir commented 8 months ago

The relevant commit touts a performance optimization in the paintEvent function whereby events obscured by the left palette are not drawn. Further investigation revealed that closing the left panel also fixes above issue. No artifacts occur with the left sidepanel closed. Conversely, they reappear immediately after the left sidepanel is opened again.

Vekhir commented 8 months ago

Not sure if connected, but making the ruler itself very large (>250cm) results in artifacts even at zoom levels <= 1. The maximum (1060cm) is enough to reproduce artifacts for very small zoom levels. These artifacts, however, only appear in the upper half of the visible board, irrespective of zooming, and only if the left side is inside the left panel. Once again, close the sidepanel and the issue goes away. At this point I have to hope that you can reproduce the issue, I can't really explain what I'm seeing. The protractor also has quite interesting behaviour if made larger than the viewport and then rotating...

letsfindaway commented 8 months ago

You add some interesting observations to an issue we already tried to mitigate. See

And to go further back in history: The commit you mention is part of

The possible glitches have been mentioned here: https://github.com/letsfindaway/OpenBoard/issues/118#issuecomment-1322089871.

An interesting point I have not found so far:

Interestingly, if I start a recording, then the artifacts stop occurring. The already existing ones stay until some other object comes near them, but no new artifacts are produced. This is true even after the recording is stopped - even if it is only a second long - and the podcast feature is disabled again (clicking "Openboard" > "Podcast" again). Was it just a moment ago basically impossible not to leave artifacts, it's now incredibly difficult to produce them. I think I still got one small artifact afterwards, but they are essentially gone. Restarting OpenBoard makes the artifacts occur again.

During investigation of the options for the thumbnails we also stumbled across https://github.com/letsfindaway/OpenBoard/issues/118#issuecomment-1327513222: Connecting something to QGraphicsScene::changed is a huge performance problem, as it changes the way how scene updates are processed internally. And here we have the connection to recording: When starting a recording we actually do such a connect:

https://github.com/OpenBoard-org/OpenBoard/blob/1420d5f36cb9188081b6b9bccaeedcb1f0e97f8e/src/podcast/UBPodcastController.cpp#L486-L487

And disconnecting later does not revert this effect. So it seems that this alternative low-performing rendering method avoids some problem producing the glitches. Using an alternative notification mechanism for recording is something which is also somewhere on my list, but not with high priority at the moment.

I have some work in progress again for fixing the glitches issue and have now pushed that branch:

https://github.com/letsfindaway/OpenBoard/tree/fix-live-thumbnail-update

It could help if you could test that.

Vekhir commented 8 months ago

The thumbnail image gets a bit choppy when moving stuff (there is a small but noticable lag), but the artifacts are gone and that's the more important part.

letsfindaway commented 8 months ago

The thumbnail image gets a bit choppy when moving stuff (there is a small but noticable lag), but the artifacts are gone and that's the more important part.

That's due to the holdoff timer of 100ms I introduced to mitigate the performance problem. Performance is the other point I want to have tested. It's not an issue on my computer -- seems to be too powerful even on a single thread, but what was reported by @kaamui and led to reverting the previous attempt.

Of course we can choose another value for that timer. It's just a tradeoff.

Vekhir commented 8 months ago

Performance is the other point I want to have tested

With the fix, it's not as smooth, though not sure if that would be confirmed by a blind test. Perhaps my machine is also too powerful, even after putting it into powersave. From my naive perspective, how does updating the thumbnail cause glitches on the drawing board? I'm going off https://github.com/letsfindaway/OpenBoard/issues/152#issuecomment-1790989025 which states:

Line 1218 [in UBThumbnailWidget.cpp] causes the glitches we wanted to avoid

Or are those other glitches? That seems to me like the reason for the timer. If we could get rid of the timer, then that would be great, wouldn't it? Sorry if it's already explained somewhere

letsfindaway commented 8 months ago

No, these are exactly those glitches. And I cannot explain it fully to you, because I don't understand it completely. My impression is the following:

Vekhir commented 8 months ago

Qt has SceneLayers which apparently are drawn independently from each other. You surely are aware of this, because OpenBoard uses them for the background. They also have a foreground layer which is drawn last. I guess that's used for HUDs and stuff. Aren't the palletes kinda HUDs? Moving them into that foreground layer by drawing them inside drawForeground might just prevent that update issue you are describing and we are experiencing. Not sure how intelligently Qt can handle the obscured by the foreground part of the drawing board, so performance would need to be tested, but from a theoretical standpoint, I'd think that it's better than our manual timer and enabling/disabling updates. What do you think?

letsfindaway commented 8 months ago

Qt has SceneLayers which apparently are drawn independently from each other. You surely are aware of this, because OpenBoard uses them for the background.

The scene layers are actually not used by OpenBoard. It manages its own Z-level management of items to control drawing of background, items, and stuff like the eraser circle in the foreground. This is a bigger task to change that, even if I think that it will be necessary at some point in time. Currently OpenBoard controls rendering using the obsolete function QGraphicsScene::drawItems, e.g. to draw things like the eraser circle on the control screen, but not on the display screen (projector).

Edit and side note: Interestingly, this function is deprecated already with Qt 5, but still available in Qt 6. For me this is an indicator that quite some people are using it and they cannot easily remove that function without offering an adequate alternative.

They also have a foreground layer which is drawn last. I guess that's used for HUDs and stuff. Aren't the palletes kinda HUDs? Moving them into that foreground layer by drawing them inside drawForeground might just prevent that update issue you are describing and we are experiencing.

The palette (as all OpenBoard palettes) is not part of the scene. It is a separate widget displayed on top of the board widget. I think the palette cannot be part of the foreground of the scene. For the foreground, you just have a QPainter, But the palette is a complex thing with a scrollbar, DnD of pages, sometimes buttons with actions, etc which cannot just be painted. BTW: what is "HUD" standing for?

Not sure how intelligently Qt can handle the obscured by the foreground part of the drawing board, so performance would need to be tested, but from a theoretical standpoint, I'd think that it's better than our manual timer and enabling/disabling updates. What do you think?

But agreed, the current "solution" is just a hack and workaround. A cleaner solution would be appreciated. However it is difficult to achieve as long as we don't exactly know what the reason is.

Vekhir commented 8 months ago

Currently OpenBoard controls rendering using the obsolete function QGraphicsScene::drawItems,

This sent me down a rabbit hole where I found the setting QGraphicsView::ViewportUpdateMode. I saw that the setting QGraphicsView::SmartViewportUpdate is used in src/board/UBBoardView.cpp. Just for fun, I replaced that with QGraphicsView::FullViewportUpdate (i.e. always redraw everything, no optimizations) and no artifacts occur!

I also tested with the other ones (MinimalViewportUpdate, BoundingRectViewportUpdate) and they all produced artifacts. So apparently, there is an issue in the viewport update where Qt cannot figure out the area that actually needs redrawing. Did you know that already? If anything, using QGraphicsView::FullViewportUpdate at least looks like a cleaner workaround than the timer stuff. The performance concern remains, though maybe we can figure out which optimization is at fault here and carry the patch ourselves. The behaviour applies both to Qt5 and Qt6.

Edit: HUD means heads-up display, like in games. Essentially an overlay.

letsfindaway commented 8 months ago

Your findings also indicate that the problem has to do with some Qt internal optimization of rendering. But you found a workaround which is closer to the actual cause and therefore actually cleaner than mine.

My way to at least estimate the CPU usage is now to start top in a console, to enter O COMMAND=openboard and s 0.5 to filter the process and increase the update rate.

I can then see that FullViewportUpdate uses remarkably more CPU, e.g. when just drawing. That's what I expected. But probably we can also make this a little bit more intelligent: To my understanding the problem only occurs with the Selector and the Play tool. In my fix I was using this information to set the affectsWholeScene flag. Perhaps we can use this information to switch update modes depending on the current tool?

And another observation: I'm often using the Horologe to test performance of widgets, because this application does a regular update. I tested a board with two Horologes and some drawing. Here I even get better performance, less CPU with the FullViewportUpdate when I just leave the board alone. I think we have to consider and test several scenarios.

Vekhir commented 8 months ago

I think at this point it becomes necessary to create a minimal working example that we can give the Qt people so they can figure out which optimization is causing the artifacts.

With regards to performance, perhaps @kaamui can test how much replacing QGraphicsView::SmartViewportUpdate with QGraphicsView::FullViewportUpdate in src/board/UBBoardView.cpp impacts performance and whether the impact is tolerable.