Skip to content

Draft: 💍 One Ring to Bind all Preferences

Amy spark requested to merge lsegovia/krita:work/amyspark/kisconfig into master

This MR is a draft proposal for one-ringing (à la Tolkien) the preferences usage we have throughout Krita.

One of the key concerns voiced by @rempt since I started collaborating with Krita was "we don't have a single source of truth!". Back when I did the G'MIC plugin-i-fication in !581 (merged), this bit us hard because G'MIC did its own changes to QCoreApplication. This issue, and more, was documented in T14815. I started working on it a year ago, but left it at that for lack of reliable tooling to trace all the usages of preferences. Thanks to @dragonmux's mentoring, and my clangd/GCC/MSVC toolkits, I could go back to it and tackle this whole!

The core of this MR is a superclass of KisConfig, KisConfigBase. KisConfigBase is intended, once ready and verified, to serve as a lightweight, fully PIMPL'd abstraction to whatever backend we might use in the future.

But the refactor doesn't stop there. As part of the refactor, I have:

  • removed the mutability of KisConfig
  • set proper compiler-verifiable constness for all the setters
  • audited all usages of KisConfig and, where possible, replaced with cheaper const KisConfigBase instances
  • removed the possibility of initializing non-const, read-only instances. (this will now trigger a hard assertion!)
  • introduce the new abstraction to the GUI controls
  • cleaned up all inclusions of KSharedConfig, KConfigGroup, KConfig, and KisConfig (in case this one wasn't obvious 😅)

This MR was created as a draft and without a milestone because it is a really invasive change.

So please bring the pitchforks, torches, and chainsaws, and let's review it!

Questions (that I remember right now)

  • We use a whole lot of read-write KConfigGroup/KSharedConfig instances, I guess to sidestep KisConfig, but the tradeoff is that there are no obvious semantics as to the sync() calls.

    • Do you intend to force a sync() to storage on each destruction, or just rely on KConfig's synchronization? (Which, BTW, is a no-op on all platforms except for Linux due to its reliance on D-Bus.)

  • The controls that were forked from KDE (mainly those in xmlgui) willy-nilly instance the configuration and then feed it back to KDE's innards, which breaks the concern-separation model I'm using for this refactor.

    • Do you think we should put the elbow grease and subclass KConfigBase for these cases?

  • Key features that lock us in to KDE at present are:

    • a clear synchronization mechanism
    • T <-> QVariant automatic conversion

    QSettings, conversely, let us use whatever the underlying OS provides, and presents a fully cross-platform, sync-able abstractions.

    • Do you think we should put the elbow grease to remove our reliance on both the above items?

Test Plan

Build Krita. Run tests and use Krita.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

/cc @rempt

Merge request reports