D8917: Reduce the amount of spurious property changes on ColorScope

2018-02-12 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > colorscope.h:131 > +QObject *const m_parent; > +Plasma::Theme::ColorGroup m_actualGroup; > valgrind says this member isn't initialized, please fix. http://www.davidfaure.fr/2018/colorscope_valgrind_log.txt REPOSITORY R242 Plasma

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-02 Thread Aleix Pol Gonzalez
This revision was automatically updated to reflect the committed changes. Closed by commit R242:f8047e86b298: Reduce the amount of spurious property changes on ColorScope (authored by apol). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 23233. apol added a comment. Fix david's comment REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8917?vs=23202=23233 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8917 AFFECTED FILES

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread Marco Martin
mart added a comment. ah, you are right, yes, it should do checkcologgroupchanged instead REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D8917 To: apol, #plasma, mart, davidedmundson Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot,

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > mart wrote in colorscope.cpp:193 > this is when the item changes window and we're not sure we are still in the > same color set, so i think is ok to keep this signal yeah, my point was we may /also/ need the colorsChanged signal (via

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread Marco Martin
mart accepted this revision. mart added inline comments. INLINE COMMENTS > davidedmundson wrote in colorscope.cpp:193 > check here. this is when the item changes window and we're not sure we are still in the same color set, so i think is ok to keep this signal REPOSITORY R242 Plasma

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread David Edmundson
davidedmundson added a comment. description needs updating with the new benefits (saving lookups every time) INLINE COMMENTS > colorscope.cpp:193 > if (value.window) { > emit colorGroupChanged(); > } check here. REPOSITORY R242 Plasma Framework (Library)

D8917: Reduce the amount of spurious property changes on ColorScope

2017-12-01 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 23202. apol added a comment. Move to tracking parents instead of doing a look-up on every color get REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8917?vs=22664=23202 BRANCH master REVISION DETAIL

D8917: Reduce the amount of spurious property changes on ColorScope

2017-11-30 Thread David Edmundson
davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. Concept is good. INLINE COMMENTS > colorscope.cpp:127 > } > +m_lastGroup = m_group; > return m_group; It's weird to be caching in a public getter.

D8917: Reduce the amount of spurious property changes on ColorScope

2017-11-20 Thread Aleix Pol Gonzalez
apol created this revision. apol added reviewers: Plasma, mart. Restricted Application added projects: Plasma, Frameworks. Restricted Application added subscribers: Frameworks, plasma-devel. REVISION SUMMARY At the moment whenever something changed we were emitting colorGroupChanged and then