Investigate stabilizer and stabilizer delayed paint helper timing issues
Closed, ResolvedPublic

Description

Ref: https://bugs.kde.org/show_bug.cgi?id=369349
Ref: T4182

Investigate the timing related to KisStabilizedEventsSampler and KisStabilizerDelayedPaintHelper. The aim is to improve the smoothness of the visual feedback of the stabilizer, and also improve the timing behaviour of the actual stabilizer if possible.

More timing logs and also include graphs showing the timing and number of points



alvinhochun added a comment.EditedJul 3 2017, 7:45 PM

QTimer uses Qt::CoarseTimer by default

It turns out that on Windows, if the timer interval is less than 20ms, it automatically uses the same timer as Qt::PreciseTimer.

Qt 5.6.1: https://github.com/qt/qtbase/blob/v5.6.1/src/corelib/kernel/qeventdispatcher_win.cpp#L612

That's why if stabilizerDelayedPaintInterval is set to 16 it would appear much smoother than when set to 20. It's not the difference between 60Hz and 50Hz, but instead 60Hz vs around 30~40Hz

I am considering whether to have KisStabilizedEventsSampler switch to use Qt::PreciseTimer for higher timer precision on Windows.

However, since the original timer had pretty much always been firing later than the specified interval (stabilizerSampleSize , which is 50ms by default and would get delayed by 10ms), this might subtly change the behaviour of the stabilizer, because the time interval will be shorter now.

  • From my timing logs, when using Qt::CoarseTimer, the timer would occasionally get delayed by an extra 50ms which might produce irregular results.
  • With Qt::PreciseTimer I didn't get any delays over 5ms, *however* it would try to catch up after small delays which makes certain intervals a bit less than 50ms, which might in turn also cause irregularities. If the system is weak or is under heavy load, I'm not certain that it wouldn't keep trying to catch up and degrade the system performance even more.

I can't really decide, but since no users seem to complain about this, it might be sufficient to just keep Qt::CoarseTimer?

alvinhochun updated the task description. (Show Details)Jul 7 2017, 6:50 AM

I can probably rewrite KisStabilizedEventsSampler and KisToolFreehandHelper::stabilizerPollAndPaint to not use a timer but instead do it during KisToolFreehandHelper::paint, but it might not be a good idea to block the pointer event processing for too long... ATM KisToolFreehandHelper::getStabilizedPaintInfo seems pretty heavy (takes 4ms from my tests).

abrahams added a subscriber: abrahams.EditedJul 7 2017, 6:54 PM

It seems strange that getStabilizedPaintInfo would be some sort of choke point. If that's really the case, I wonder if it would be worth pushing the implementation of getStabilizedPaintInfo into KisPaintInformation using a function with signature like
KisPaintInformation::mixQueue(QQueue<KisPaintInformation> queue, bool stabilizeSensors).

There seems to be a lot of unnecessary copying with all these KisPaintInformation objects getting constructed and destroyed. There are also a bunch of cross-library function calls between kritaimage and kritaui which the compiler may not be optimizing well. At the end of the day this loop is simply calculating a few moving averages, and lot of the extra information that gets copied into the intermediate KisPaintInformation objects (canvasRotation, randomSource etc. etc.) only needs to be copied once into the final result.

I would try writing this myself but my linux partition is long broken...

It seems strange that getStabilizedPaintInfo would be some sort of choke point. If that's really the case, I wonder if it would be worth pushing the implementation of getStabilizedPaintInfo into KisPaintInformation using a function with signature like
KisPaintInformation::mixQueue(QQueue<KisPaintInformation> queue, bool stabilizeSensors).

There seems to be a lot of unnecessary copying with all these KisPaintInformation objects getting constructed and destroyed. There are also a bunch of cross-library function calls between kritaimage and kritaui which the compiler may not be optimizing well. At the end of the day this loop is simply calculating a few moving averages, and lot of the extra information that gets copied into the intermediate KisPaintInformation objects (canvasRotation, randomSource etc. etc.) only needs to be copied once into the final result.

I would try writing this myself but my linux partition is long broken...

With some profiling it seems that the major overhead comes from the heap allocations made by KisPaintInformation (PIMPL private data). So yeah, it is related to the repeated construction and destruction of KisPaintInformation.
But I also made the abomination which is D6522 to experiment with.

I'll try your suggestions and see what differences it'd make.

alvinhochun closed this task as Resolved.Aug 1 2017, 4:17 PM