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