romangg added inline comments. Restricted Application edited projects, added KWin; removed Plasma.
INLINE COMMENTS > blur.cpp:35 > > +#define BORDER_SIZE 5 > + `static const` and in the KWin namespace please. Also camel case then. > blur.cpp:139 > + * > + * The texture blur amount is depends on the downsampling iterations and > the offset value. > + * By changing the offset we can alter the blur amount without relying > on further downsampling. The texture blur amount depends > blur.cpp:152 > + * have good minimum and maximum blur amount, but there would be around > only 5 blur strength > + * steps. > + * This means we can't have constant offset value. I think 5 blur strength steps to select from is fine, if that means we can get rid off the magic numbers. Other opinions? > blur.cpp:180 > + m_blurConfigData.append({4, 7, 170}); > + m_blurConfigData.append({4, 8, 200}); > +} All this data is constant, so it doesn't make sense to reinitialize it on every Blur instantiation and it is only a helper for reconfigure. Maybe instead as static const in the file? See here how to do it for a QVector: https://forum.qt.io/topic/32353/solved-creating-a-const-qvector-with-values > blur.cpp:212 > + > + int blurStrength = BlurConfig::blurStrength(); > + m_downSampleIterations = > m_blurConfigData[blurStrength].downSampleIterations; You need to guard against out of bounds values. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D9848 To: anemeth, #plasma, #kwin Cc: romangg, zzag, anthonyfieroni, mart, davidedmundson, fredrik, ngraham, plasma-devel, kwin, #kwin, iodelay, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol