D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

rjvbb added a comment. A little tinker tool: github.com/RJVB/kbusygadget. An inter-frame freeze duration of 75ms already decreases CPU load (according to `top`) to approx. 3.6% (= almost 4x). I cannot really say if I notice the effect of this short a freeze on the rotation

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

rjvbb added a comment. And my point is that you are doing 720 translations and 360 rotations per cycle, with subsequent smoothing of an image, continuously and with sufficient temporal resolution to get a fluid animation that is completely overkill here. Indicating a busy state (a two-state

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added a comment. My point is that your lamentations have nothing to do with this class but with Q*Animation on your system. So you need to find out what's wrong and talk to Qt. I am 100% against a workaround that degrades the user experience when the bug isn't even in this class.

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

rjvbb added a comment. In D22375#541399 , @sitter wrote: > The demo doesn't even use this widget That wasn't the question I answered by referring to oxygen-demo > breeze > F7506213: Peek 2019-10-03 13-46.gif

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added a comment. The demo doesn't even use this widget breeze F7506213: Peek 2019-10-03 13-46.gif oxygen F7506215: Peek 2019-10-03 13-45.gif REPOSITORY R236 KWidgetsAddons REVISION DETAIL

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

rjvbb added a comment. > Does it happen with every code that uses QPropertyAnimation, or just with this KBusyIndicator? I don't know, neither for QVariantAnimation (which is used here). Testing just now (on the N3150 machine) with the Sliders page of the oxygen-demo app I get

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added a comment. F7505620: Peek 2019-10-03 12-23.gif REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22375 To: sitter, cfeck, apol Cc: rjvbb, ngraham, kossebau, broulik, kde-frameworks-devel, apol, LeGast00n,

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck added a comment. Does it happen with every code that uses QPropertyAnimation, or just with this KBusyIndicator? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22375 To: sitter, cfeck, apol Cc: rjvbb, ngraham, kossebau, broulik, kde-frameworks-devel,

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

rjvbb added a comment. I'll repeat here what I muttered on the associated commit page: This widget adds a lot of CPU overhead, too much IMHO: the dedicated test tool runs at a bit over 10%CPU, and that is not counting the additional overhead from the displaying layers (X server, the Mac

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

This revision was automatically updated to reflect the committed changes. Closed by commit R236:2631be903f94: new class KBusyIndicatorWidget similar to QtQuicks BusyIndicator (authored by sitter). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck accepted this revision. REPOSITORY R236 KWidgetsAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D22375 To: sitter, cfeck, apol Cc: ngraham, kossebau, broulik, kde-frameworks-devel, apol, LeGast00n, sbergeron, michaelh, bruns

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter updated this revision to Diff 61789. sitter added a comment. typos-- REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22375?vs=61659=61789 BRANCH master REVISION DETAIL https://phabricator.kde.org/D22375 AFFECTED FILES

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck added inline comments. INLINE COMMENTS > kbusyindicatorwidget.h:52 > + * > + * @since 5.60.0 > + */ 5.61.0 > kbusyindicatorwidgettest.cpp:43 > + > +auto toggle = new QPushButton(QStringLiteral("Toggle Visibile"), > ); > + typo REPOSITORY R236 KWidgetsAddons BRANCH master

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter updated this revision to Diff 61659. sitter added a comment. - add simple example code + image - add @since tag - override event REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22375?vs=61603=61659 BRANCH master REVISION DETAIL

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

apol accepted this revision. apol added a comment. This revision is now accepted and ready to land. LGTM REPOSITORY R236 KWidgetsAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D22375 To: sitter, cfeck, apol Cc: ngraham, kossebau, broulik, kde-frameworks-devel, apol,

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter updated this revision to Diff 61603. sitter added a comment. get rid of need for decl_hidden and extend the test to play with visiblity REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22375?vs=61522=61603 BRANCH master REVISION DETAIL

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added inline comments. INLINE COMMENTS > broulik wrote in kbusyindicatorwidget.cpp:62 > Is that legal in the C++ standard allowed by frameworks? Well, it builds at least under c++11 which is what kf5 is compatible with. According to this page

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

kossebau added inline comments. INLINE COMMENTS > kbusyindicatorwidget.h:40 > + * is more specific. > + */ > +class KWIDGETSADDONS_EXPORT KBusyIndicatorWidget : public QWidget Any chance of getting some samples how this class is supposed to be used? Sounds one should show & hide the complete

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter updated this revision to Diff 61522. sitter marked 14 inline comments as done. sitter added a comment. - running property gone since we have no use case + simplified code since we no longer account for it - dropped superfluous virtual keywords - bumped duration to 1.5s - moved to

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck added inline comments. INLINE COMMENTS > sitter wrote in kbusyindicatorwidget.h:48 > Mimics the QML API, haven't given it any thought TBH. You can have the > spinner visible but paused, I am not sure why exactly you'd want a paused > spinner but that's what the property does anyway. > >

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added inline comments. INLINE COMMENTS > broulik wrote in kbusyindicatorwidget.cpp:111 > How about `PM_LargeIconSize` Oh, that would work! > cfeck wrote in kbusyindicatorwidget.h:48 > Why is this property needed? If the (parent) widget is no longer busy, this > spinner needs to be

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck added inline comments. INLINE COMMENTS > kbusyindicatorwidget.h:48 > + */ > +Q_PROPERTY(bool running READ running WRITE setRunning NOTIFY > runningChanged) > +public: Why is this property needed? If the (parent) widget is no longer busy, this spinner needs to be hidden anyway.

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

cfeck added a comment. Ideally you can create it with any size as an overlay to an existing widgets (also to block input there), but the spinner itself is only rendered at a smaller centered position. REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D22375

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

broulik added inline comments. INLINE COMMENTS > sitter wrote in kbusyindicatorwidget.cpp:111 > I was thinking about it, and honestly I am not sure. I don't think we have > access to anything to do with icon units, so we could hardcode some > dimensions (22,22 I guess) but that's about it.

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter added inline comments. INLINE COMMENTS > broulik wrote in kbusyindicatorwidget.cpp:111 > Does this widget need a `sizeHint` of a button or default icon size or > something? I was thinking about it, and honestly I am not sure. I don't think we have access to anything to do with icon

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

broulik added a comment. Cool! INLINE COMMENTS > kbusyindicatorwidget.cpp:28 > + > +class KBusyIndicatorWidget::Private : public QObject > +{ `Q_DECL_HIDDEN` > kbusyindicatorwidget.cpp:31 > +Q_OBJECT > +Q_PROPERTY(qreal rotation MEMBER rotation WRITE setRotation) > +public:

D22375: new class KBusyIndicatorWidget similar to QtQuick's BusyIndicator

sitter created this revision. sitter added a reviewer: cfeck. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sitter requested review of this revision. REVISION SUMMARY this mimics QQC's BusyIndicator and more specifically our styling of it. KBIW loads