a definite +1

looks a lot better and i really havent noticed any difference in speed/performance (i've lost barely 2 fps on low end hardware so... negligible imo)

Wow, it certainly does look better! I know the example image is rather contrived to show the effect, but that effect is quite dramatic!

Good Lord, somehow I managed to get this review lost in my pile of emails.

What we are facing here is a classical problem of forgetting to convert non-linear sRGB to linear RGB and vice versa. In our case this level of inaccuracy is acceptable, KWin is not an image editor after all, so we don't have to be that accurate (though that's a desirable property).

Also, I don't think that's a right way to convert non-linear sRGB to linear RGB.

I suggest to leave the blur effect as it is.

+1

In D18377#398675, @zzag wrote:

I suggest to leave the blur effect as it is.

The visual effect is pretty hugely improved IMHO; could you elaborate on what changes @anemeth would need to make to render this patch merge-worthy?

In D18377#398737, @ngraham wrote:

The visual effect is pretty hugely improved IMHO; could you elaborate on what changes @anemeth would need to make to render this patch merge-worthy?

Well, the blur effect should correctly convert sRGB to linear RGB, though I personally think the blur effect is good enough already. Also, I'd like to point out that an option to enable/disable the "accurate" blur is a no-go.

In D18377#398748, @zzag wrote:

I personally think the blur effect is good enough already.

It's definitely good already, but as the "before" screenshots point out, it suffers from a well-known and common problem that the area between two blurred color appears too dark with many blur implementations. This is a bug, and fixing it is basically a bugfix.

Also, I'd like to point out that an option to enable/disable the "accurate" blur is a no-go.

I agree that an option is undesirable.

In D18377#398674, @zzag wrote:

Also, I don't think that's a right way to convert non-linear sRGB to linear RGB.

Okay, it looks like it's an approximation.

In D18377#398675, @zzag wrote:

I suggest to leave the blur effect as it is.

This blur affects some KDE parts, like KWin's edge shadow:

Hmm, it looks like during each render pass we'll be wasting resources on "sRGB - linear RGB" conversions. Intermediate results should be in linear colorspace.

In D18377#398810, @rapiteanu wrote:

This blur affects some KDE parts, like KWin's edge shadow:

In the previous example the shadow should transition from transparent to light blue, not a dark dark brown shape with a shade of blue.

This is unrelated.

In D18377#398821, @zzag wrote:

Hmm, it looks like during each render pass we'll be wasting resources on "sRGB - linear RGB" conversions. Intermediate results should be in linear colorspace.

The hardware will do these conversions automatically if the texture format is GL_SRGB8_ALPHA8, and GL_FRAMEBUFFER_SRGB is enabled.

In D18377#398675, @zzag wrote:

I suggest to leave the blur effect as it is.

the specifics need to be worked out, but leaving the blur effect as it is right now is a mistake

it's really unnatural and makes an inordinate number of people (even in the VDG group) shy away from dark themes and lower opacity values (with this blur effect in place, dark themes look a lot more natural)

In D18377#400095, @rooty wrote:

it's really unnatural

Is it really that unnatural? Is it really very noticeable? We haven't received any bug reports about that. In fact people praised the new blur effect. This issue was "noticed" only after a post on reddit. Though I'd like to point out that the rabbit hole goes much deeper. For example, Firefox/Google Chrome/Gwenview/Eye of GNOME/etc don't properly scale down images. Does it make the scaled down images look really unnatural? No.

Yes, that's not good to blur textures without converting colors to the linear color space, but please don't exaggerate the whole situation. Let's think rationally.

shy away from dark themes

That's not true.

Also, I'd like to point out that some of my comments above are no longer actual because Fredrik proposed more simpler solution. :-)

I tried experimenting what fredrik suggested without luck so far.
@fredrik @zzag is this the correct approach? This gives the exact same result as not using sRGB textures.

In D18377#400171, @anemeth wrote:

I tried experimenting what fredrik suggested without luck so far.
@fredrik @zzag is this the correct approach? This gives the exact same result as not using sRGB textures.

Basically yes, but it's important to keep in mind that the framebuffer contents are effectively sRGB even though the framebuffer format is not.
So in other words, GL_FRAMEBUFFER_SRGB must be disabled when doing the initial copy from the framebuffer to the first sRGB texture, so the gamma curve is not applied twice.

@fredrik swoops in and solves every problem!
how it looks with this diff:

In D18377#400281, @fredrik wrote:

Basically yes, but it's important to keep in mind that the framebuffer contents are effectively sRGB even though the framebuffer format is not.
So in other words, GL_FRAMEBUFFER_SRGB must be disabled when doing the initial copy from the framebuffer to the first sRGB texture, so the gamma curve is not applied twice.

May it cause issues on GLES?

hey guys so i don't understand any of what's being talked about here but i've noticed that with this latest patch, latte-dock turns very very dark, could this be a bug in latte-dock unrelated to this patch?

before:

(the opacity is set to 50%, if i lower it to 5% it looks just as dark)

konsole's blur looks a lot better though
blur elsewhere varies - the panel looks almost identical to what it looked like before, except slightly darker, and the popups look the same as before (hence there's more of a contrast between panel and popup):

before:

@rooty please test it again with this

In D18377#400445, @zzag wrote:

May it cause issues on GLES?

There are a number of issues with GLES unfortunately:

• You have to enable sRGB rendering by specifying { EGL_GL_COLORSPACE_KHR, EGL_GL_COLORSPACE_SRGB_KHR } in the eglCreateWindowSurface() attributes.
• GL_FRAMEBUFFER_SRGB is enabled by default, and can only be toggled when GL_EXT_sRGB_write_control is supported.
• On GLES2 you also need GL_EXT_sRGB.

Given all this, it's probably best to limit this to desktop GL for now.

In D18377#400816, @anemeth wrote:

@rooty please test it again with this

excellent - it works perfectly now

In D18377#400821, @fredrik wrote:

Given all this, it's probably best to limit this to desktop GL for now.

you mean just put a bunch of "if opengles do this else do that" in it?

what the important or blur on plasma when no use of it on any theme or application by default

In D18377#400869, @Codezela wrote:

what the important or blur on plasma when no use of it on any theme or application by default

almost all the themes and a small but substantial number of applications do...

In D18377#400871, @rooty wrote:

In D18377#400869, @Codezela wrote:

what the important or blur on plasma when no use of it on any theme or application by default

almost all the themes and a small but substantial number of applications do...

i know that but i mean breeze the official theme
and kde applications
we need yo use this magnificent blur effect
better than that
i hope we do this

This revision is not a place to discuss where the blur effect has to be used. Please leave comments that are related to this change.

In D18377#400909, @zzag wrote:

This revision is not a place to discuss where the blur effect has to be used. Please leave comments that are related to this change.

sorry for that

@zzag @fredrik what do you think now?

Updated the summary and the screenshots

Please re-title the change, it should look something like "[effects/blur] ...". Also, please remove the video link from the summary.

In D18377#402740, @zzag wrote:

Please re-title the change, it should look something like "[effects/blur] ...". Also, please remove the video link from the summary.

done

I am using Plasma 5.16, this patch probably can create TOO DARKISH results so the natural part of the commit is not valid and probably breaks users experience also:

Konsole fully transparent:

My transparent Konsole is also noticeably darker now, but it actually seems to me like the background color of the Konsole color scheme got somehow changed. It doesn't match the color in the regular color scheme anymore.

In D18377#480680, @filipf wrote:

My transparent Konsole is also noticeably darker now, but it actually seems to me like the background color of the Konsole color scheme got somehow changed. It doesn't match the color in the regular color scheme anymore.

I get exactly the same darkish results with Latte Docks that are fully transparent but have enabled Blur, this is probably a bug in Blur effect

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it
I can't reproduce the issue

In D18377#480678, @mvourlakos wrote:

I am using Plasma 5.16, this patch probably can create TOO DARKISH results so the natural part of the commit is not valid and probably breaks users experience also:

Konsole fully transparent:

Are you sure this problem is not caused by using a dark background color in Konsole? It makes sense that the blurred area would be dark unless you set the transparency to 100%.

Could you please try lowering the noise strength all the way down?
Noise caused some problems very similar to this a few months ago but I """fixed it"""

In D18377#480776, @anemeth wrote:

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it

You need an Intel graphics card to reproduce this bug.

In D18377#480824, @zzag wrote:

In D18377#480776, @anemeth wrote:

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it

You need an Intel graphics card to reproduce this bug.

Yep, I am at Intel driver, scale is 1x, noise at blur is at minimum and problem still occurs

https://bugs.kde.org/show_bug.cgi?id=408790 I am not the only one with issue, the issue was reported also from Latte users that probably use Intel driver

In D18377#480824, @zzag wrote:

In D18377#480776, @anemeth wrote:

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it

You need an Intel graphics card to reproduce this bug.

That's strange. I have Intel HD Graphics 520 and I'm not seeing this bug.

In D18377#480824, @zzag wrote:

In D18377#480776, @anemeth wrote:

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it

You need an Intel graphics card to reproduce this bug.

Yeah, the problem is probably that the GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING is GL_LINEAR with the Intel driver, and that means that the final conversion from linear to sRGB is not happening in the upscaleRenderToScreen() pass.

We should probably not use sRGB textures when the driver reports that the framebuffer color encoding is linear.

However, it might also be a bug in the driver that it doesn't pick an sRGB format for the default framebuffer. We should discuss that with the Intel developers, but I would appreciate it if someone who has an Intel GPU could investigate this a bit further first. At the very least confirm that glGetFramebufferAttachmentParameteriv() in fact returns GL_LINEAR.

In D18377#480985, @fredrik wrote:

In D18377#480824, @zzag wrote:

In D18377#480776, @anemeth wrote:

I just downloaded the KDE Neon 5.16 iso from the website, installed it and updated it

You need an Intel graphics card to reproduce this bug.

Yeah, the problem is probably that the GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING is GL_LINEAR with the Intel driver, and that means that the final conversion from linear to sRGB is not happening in the upscaleRenderToScreen() pass.

We should probably not use sRGB textures when the driver reports that the framebuffer color encoding is linear.

However, it might also be a bug in the driver that it doesn't pick an sRGB format for the default framebuffer. We should discuss that with the Intel developers, but I would appreciate it if someone who has an Intel GPU could investigate this a bit further first. At the very least confirm that glGetFramebufferAttachmentParameteriv() in fact returns GL_LINEAR.

I can test if you want:

1. Is there any command that can provide that information?
2. Do you have an small testing app that I can compile and return to you the output?

Please discuss this issue in https://bugs.kde.org/show_bug.cgi?id=408594, not in this review request.