broulik added a comment.
  Pretty cool idea!

INLINE COMMENTS

> WallpaperDelegate.qml:73
> +
> +            GaussianBlur {
> +                visible: cfg_Blur

I think for the preview a `FastBlur` is sufficient, or perhaps even just 
scaling up a tiny pixmap

> config.qml:142
> +            id: blurCheckBox
> +            text: i18nd("plasma_applet_org.kde.image", "Use blur background 
> filling")
> +            checked: true

I'm not too happy with this label but I can't think of a better phrase either..

> main.qml:213
>  
>      Image {
> +        id: blurBackgroundSource

Is it possible to re-use the currentImage instead of creating yet another 
`Image` item? I know with `ShaderEffect` you can do `hideSource: false` but I 
don't see this in `GaussianBlur` :/

> main.qml:226
> +        anchors.fill: parent
> +        source: blurBackgroundSource
> +        radius: 32

You might want to set the source to `null` when it's disabled otherwise we end 
up blurring (even when disabled):

  source: wallpaper.configuration.Blur ? blurBackgroundSource : null

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D7047

To: guoyunhe, #plasma_workspaces
Cc: broulik, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart, lukas

Reply via email to