D9848: Updated the blur method to use the more efficient dual kawase blur algorithm.

[Roman Gilg]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180114/c0b19d4a/attachment.html>