[graphics/krita] libs/ui: Reduce delay of Overlay, Histogram and Layer Thumbnails updates

[Dmitry Kazakov]

HI, all!

This patch needs testing from real painters who happen to use Overview
docker during their normal work. Please pay attention if Krita starts
doing any hiccups when you paint with very quick sequential strokes (of
about 200ms) :)

Looking at the code, there should be regressions, but who knows :)

On Mon, Jul 29, 2024 at 11:26 AM Dmitry Kazakov <null at kde.org> wrote:

> Git commit 572c64858d1b506ca1500b776cc49dfd65105ddd by Dmitry Kazakov.
> Committed on 29/07/2024 at 09:26.
> Pushed by dkazakov into branch 'master'.
>
> Reduce delay of Overlay, Histogram and Layer Thumbnails updates
>
> It was reported that the update delay of the aforementioned previews
> is too slow in Krita, so this patch tries to address the issue
> by reducing the idle watcher delay to 50 ms.
>
> It seems like these days such huge delay is not really needed, because
> all the tasks are split into multithreaded jobs, which are **cancelled**
> when the user actually starts doing any sane action. When the code was
> originally written, Overview and Layer Thumbnails strokes were not
> multithreaded, so they couldn't be cancelled with decent granularity.
> Now it is not the case anymore.
>
> The patch needs testing from real painters who use Overview docker on
> a regular basis.
>
> The patch is based on the work of Zhiqi Yao, who proposed this MR:
> https://invent.kde.org/graphics/krita/-/merge_requests/2180
>
> This patch reduces the scope of the original MR to
> Overlay, Histogram and Layer Thumbnails tasks only, not changing
> the idle watcher's defaults. It also adds a small sanity check that
> would help us catching issues in the future.
>
> CC:kimageshop at kde.org
>
> M  +49   -0    libs/ui/KisIdleTaskStrokeStrategy.cpp
> M  +6    -0    libs/ui/KisIdleTaskStrokeStrategy.h
> M  +5    -0    libs/ui/KisIdleTasksManager.cpp
>
>
> https://invent.kde.org/graphics/krita/-/commit/572c64858d1b506ca1500b776cc49dfd65105ddd
>
> diff --git a/libs/ui/KisIdleTaskStrokeStrategy.cpp
> b/libs/ui/KisIdleTaskStrokeStrategy.cpp
> index 915163649e5..4ffeea30d38 100644
> --- a/libs/ui/KisIdleTaskStrokeStrategy.cpp
> +++ b/libs/ui/KisIdleTaskStrokeStrategy.cpp
> @@ -39,9 +39,30 @@ KisStrokeStrategy
> *KisIdleTaskStrokeStrategy::createLodClone(int levelOfDetail)
>      return new
> KisSimpleStrokeStrategy(QLatin1String("KisIdleTaskStrokeStrategy_FakeLodN"));
>  }
>
> +void KisIdleTaskStrokeStrategy::initStrokeCallback()
> +{
> +    m_idleStrokeTime.start();
> +}
> +
>  void KisIdleTaskStrokeStrategy::finishStrokeCallback()
>  {
>      emit sigIdleTaskFinished();
> +
> +    /**
> +     * Just a small sanity check if idle tasks don't occupy too much time
> +     * and don't interfere with user's workflow. Theoretically, even if
> the
> +     * time consumed is too high, it may still be acceptable if the task
> is
> +     * split in multiple jobs, which would be cancelled but the user's
> action.
> +     */
> +    const qint64 elapsedTime = m_idleStrokeTime.elapsed();
> +
> +    if (elapsedTime > preferredIdleTaskMaximumTime()) {
> +        qWarning() << "WARNING: idle task consumed more than" <<
> +                      preferredIdleTaskMaximumTime() <<
> +                      "ms, it can cause visible distractions to the user";
> +        qWarning() << "WARNING: time consumed in" << id() <<
> +                      "is" << elapsedTime << "ms";
> +    }
>  }
>
>  QWeakPointer<boost::none_t> KisIdleTaskStrokeStrategy::idleTaskCookie()
> @@ -50,3 +71,31 @@ QWeakPointer<boost::none_t>
> KisIdleTaskStrokeStrategy::idleTaskCookie()
>      m_idleTaskCookie.reset(new boost::none_t(boost::none));
>      return m_idleTaskCookie;
>  }
> +
> +int KisIdleTaskStrokeStrategy::preferredIdleTaskMaximumTime()
> +{
> +    /**
> +     * Sometimes (unless split into multiple jobs) idle tasks can
> interfere
> +     * with user actions. We have a sanity check that checks if the tasks
> are
> +     * quick enough to not interfere into the user's actions. This time
> limit
> +     * is the maximum "allowed" idle task size.
> +     *
> +     * Definition of "allowed" here is not strict, since the tasks are not
> +     * cancelled when reaching the limit. Just a warning is spit into the
> +     * terminal.
> +     */
> +
> +    return 200; // ms
> +}
> +
> +int KisIdleTaskStrokeStrategy::preferredIdleWatcherInterval()
> +{
> +    /**
> +     * Preferred idle watcher interval for checks of the image idleness
> +     * state. Idle watcher checks the image four times before starting an
> +     * idle task, so the actual idle task delay is
> +     * `4 * preferredIdleWatcherInterval()` milliseconds.
> +     */
> +    return 50; // ms
> +}
> +
> diff --git a/libs/ui/KisIdleTaskStrokeStrategy.h
> b/libs/ui/KisIdleTaskStrokeStrategy.h
> index d71c7b43a79..3532fd18a47 100644
> --- a/libs/ui/KisIdleTaskStrokeStrategy.h
> +++ b/libs/ui/KisIdleTaskStrokeStrategy.h
> @@ -13,6 +13,7 @@
>  #include <boost/none.hpp>
>  #include <kis_types.h>
>  #include <KisRunnableBasedStrokeStrategy.h>
> +#include <QElapsedTimer>
>
>  /**
>   * A base class for strategies used in "idle tasks". Such strategy
> @@ -31,7 +32,11 @@ public:
>      KisStrokeStrategy* createLodClone(int levelOfDetail) override;
>      QWeakPointer<boost::none_t> idleTaskCookie();
>
> +    static int preferredIdleTaskMaximumTime();
> +    static int preferredIdleWatcherInterval();
> +
>  protected:
> +    void initStrokeCallback() override;
>      void finishStrokeCallback() override;
>
>  Q_SIGNALS:
> @@ -39,6 +44,7 @@ Q_SIGNALS:
>
>  private:
>      QSharedPointer<boost::none_t> m_idleTaskCookie;
> +    QElapsedTimer m_idleStrokeTime;
>  };
>
>  using KisIdleTaskStrokeStrategyFactory =
> std::function<KisIdleTaskStrokeStrategy*(KisImageSP image)>;
> diff --git a/libs/ui/KisIdleTasksManager.cpp
> b/libs/ui/KisIdleTasksManager.cpp
> index 63de2cab56c..1954d1f21de 100644
> --- a/libs/ui/KisIdleTasksManager.cpp
> +++ b/libs/ui/KisIdleTasksManager.cpp
> @@ -21,6 +21,11 @@ struct TaskStruct {
>
>  struct KisIdleTasksManager::Private
>  {
> +    Private()
> +        :
> idleWatcher(KisIdleTaskStrokeStrategy::preferredIdleWatcherInterval())
> +    {
> +    }
> +
>      KisImageWSP image;
>      KisIdleWatcher idleWatcher;
>      QVector<TaskStruct> tasks;
>


-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20240729/d1313523/attachment.htm>