D22147: Better use of Qt APIs in Plasma::Theme

ndavis added a comment. In D22147#491119 , @apol wrote: > In D22147#490887 , @ndavis wrote: > > > I've compiled this with the latest commits, but the problem hasn't gone away. > > > Try

D22147: Better use of Qt APIs in Plasma::Theme

apol added a comment. In D22147#490887 , @ndavis wrote: > I've compiled this with the latest commits, but the problem hasn't gone away. Try removing `.cache/plasma-svgelements-default_v5.60.0` and

D22147: Better use of Qt APIs in Plasma::Theme

ndavis added a comment. I've compiled this with the latest commits, but the problem hasn't gone away. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D22147 To: apol, #plasma, #frameworks, mart Cc: ndavis, bruns, tcanabrava, fvogt, broulik,

D22147: Better use of Qt APIs in Plasma::Theme

apol added inline comments. INLINE COMMENTS > broulik wrote in svg.cpp:313 > Oh, I think the `static` might be the culprit, I didn't realize the `status` > and `ratio` were changing. Sorry Fixed https://commits.kde.org/plasma-framework/e3c6c2731eb272ca4f66e7836281992fb1f90e04 REPOSITORY

D22147: Better use of Qt APIs in Plasma::Theme

broulik added inline comments. INLINE COMMENTS > broulik wrote in svg.cpp:313 > When you make it `static` it would only have to compile it once Oh, I think the `static` might be the culprit, I didn't realize the `status` and `ratio` were changing. Sorry REPOSITORY R242 Plasma Framework

D22147: Better use of Qt APIs in Plasma::Theme

ndavis added a comment. @apol This patch seems to have caused a bug in the way icons are loaded for the system tray: F6965245: Screenshot_20190704_032623.png When I compile plasma-framework without this patch, the system tray looks how it should:

D22147: Better use of Qt APIs in Plasma::Theme

This revision was automatically updated to reflect the committed changes. Closed by commit R242:6c692309edf7: Better use of Qt APIs in Plasma::Theme (authored by apol). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22147?vs=60932=61009

D22147: Better use of Qt APIs in Plasma::Theme

mart accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH arcpatch-D22147 REVISION DETAIL https://phabricator.kde.org/D22147 To: apol, #plasma, #frameworks, mart Cc: bruns, tcanabrava, fvogt, broulik,

D22147: Better use of Qt APIs in Plasma::Theme

fvogt resigned from this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D22147 To: apol, #plasma, #frameworks Cc: bruns, tcanabrava, fvogt, broulik, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D22147: Better use of Qt APIs in Plasma::Theme

apol updated this revision to Diff 60932. apol marked an inline comment as done. apol added a comment. address fvogt's comment REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22147?vs=60875=60932 BRANCH arcpatch-D22147 REVISION DETAIL

D22147: Better use of Qt APIs in Plasma::Theme

apol marked 3 inline comments as done. apol added inline comments. INLINE COMMENTS > fvogt wrote in svg.cpp:317 > It really does not look that way as you're immediately using captures after > that. > > If that's really what you want (which I doubt), it should be >

D22147: Better use of Qt APIs in Plasma::Theme

bruns added inline comments. INLINE COMMENTS > tcanabrava wrote in svg.cpp:342-349 > looks a good example of code that could be written with an std::find_if You either have to sort on `bestFit.area()` first, or do the find_if in a nested loop, to find a better match (smaller one) after the

D22147: Better use of Qt APIs in Plasma::Theme

fvogt added inline comments. INLINE COMMENTS > apol wrote in svg.cpp:317 > Please note this is only to make sure the regex was properly compiled. It > isn't matching there yet. It really does not look that way as you're immediately using captures after that. If that's really what you want

D22147: Better use of Qt APIs in Plasma::Theme

apol added inline comments. INLINE COMMENTS > fvogt wrote in svg.cpp:317 > `isValid` is always true, you probably want to use `hasMatch` instead. > > This is not obvious, I only noticed this because I debugged this error before > (https://phabricator.kde.org/D17359) Please note this is only

D22147: Better use of Qt APIs in Plasma::Theme

tcanabrava added inline comments. INLINE COMMENTS > svg.cpp:342-349 > +for (const QSize : elementSizeHints) { > > if (hint.width() >= s.width() * ratio && hint.height() >= > s.height() * ratio && > (!bestFit.isValid() || >

D22147: Better use of Qt APIs in Plasma::Theme

tcanabrava added inline comments. INLINE COMMENTS > svg.cpp:339-364 > if (!elementSizeHints.isEmpty()) { > QSize bestFit(-1, -1); > > -Q_FOREACH (QSize hint, elementSizeHints) { > +for (const QSize : elementSizeHints) { > > if

D22147: Better use of Qt APIs in Plasma::Theme

fvogt requested changes to this revision. fvogt added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > svg.cpp:317 > +const auto match = sizeHintedKeyExpr.match(key); > +if (match.isValid()) { > +QString baseElementId =

D22147: Better use of Qt APIs in Plasma::Theme

apol updated this revision to Diff 60875. apol added a comment. address logic, no artifacts REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22147?vs=60840=60875 BRANCH arcpatch-D22147 REVISION DETAIL

D22147: Better use of Qt APIs in Plasma::Theme

apol marked 3 inline comments as done. apol added inline comments. INLINE COMMENTS > broulik wrote in theme.cpp:106 > Can we clean up all of this custom refcounting by using a `QSharedPointer`? Implemented with QSharedData. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D22147: Better use of Qt APIs in Plasma::Theme

apol updated this revision to Diff 60840. apol marked an inline comment as done. apol added a comment. Address kai's comments REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22147?vs=60802=60840 BRANCH master REVISION DETAIL

D22147: Better use of Qt APIs in Plasma::Theme

broulik added inline comments. INLINE COMMENTS > svg.cpp:313 > // and store them locally. > -QRegExp > sizeHintedKeyExpr(CACHE_ID_NATURAL_SIZE(QStringLiteral("(\\d+)-(\\d+)-(.+)"), > status, ratio)); > +const QRegularExpression >

D22147: Better use of Qt APIs in Plasma::Theme

apol created this revision. apol added reviewers: Plasma, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY Try not to access hashes repeatedly. QRegExp -> QRegularExpression. REPOSITORY