Skip to content

qt: fix sometimes blur effect appears weird in music artist view header

This reverts commit 2baca715.

Unfortunately it turns out that "They support atlas/sub textures" is not fully correct. I made that assumption after taking a look at the shader code, and I concluded that they simply did not bother setting supportsAtlasTextures to true from its default value false due to negligence because the shader code that samples the source texture primarily uses the coordinate Qt supplies (qt_MultiTexCoord0) where it already takes the atlas textures into account.

ShaderEffect's documentation states that:

If supportsAtlasTextures is true, coordinates will be based on position in the atlas instead.

Therefore, the coordinates supplied can be used as is with atlas textures. In fact, we (and Qt) are already doing this in various places (vec4 texel = texture(source, qt_TexCoord0)).

The blur effect shaders uses the same (pre-adjusted) coordinate for sampling the texture. So, I thought that setting supportsAtlasTextures would not have a negative impact.

However, due to the way Qt does blurring, this pre-adjusted (atlas) coordinate is offsetted and that offset is obviously specific to the blur effect and not pre-adjusted for the atlas texture (as Qt does with ShaderEffect) since they never claim to support atlas textures in blur effect, they don't bother normalizing the offset coordinate according to the atlas.

Unfortunately, this means that atlas textures must be detached from the atlas, incurring a copy, to be used as a source for the blur effect. This is not a big problem for us, because most of the places where we use the blur effect already have the source textures independent (not in the atlas). And we still benefit from QSGTextureView that it prevents incurring a copy when the source texture is not in the atlas.

  • Main view (for mini player frosted glass effect background) is an item layer (not to mention a dynamic texture). They are never put in the atlas.
  • Player background uses mipmap, currently Qt does not support mipmapped atlas textures, so all mipmapped textures are independent textures. We use mipmapping here not to prevent this issue, but we actually benefit from mipmapping in this case as we do fluent size change based on window size.
  • Music artist header background, depending on the atlas size, may be put in the atlas. This currently depends on the initial window size, if it is small, the texture is not put in the atlas and blurring works fine. This is because currently Qt determines the atlas size based on the initial window size (I guess this is from the times when they primarily targeted mobile platforms with Qt Quick, back in 2010s).

Unfortunately QQuickImage (Image) does not have a parameter to disable putting textures in the atlas. Setting mipmap as done in player would be fine, but we don't want/need mipmapping in music artist view. This does not mean much anyway, as with the revert here the shader effect would ask the texture to "remove" itself from the atlas (by copying, as per the default RHI atlas texture implementation), so blurring starts working fine.

This change brings an additional requirement, although not used currently, if a target of QSGTextureView is in the atlas, and the texture view is used as source for blur effect, the request of detaching from the atlas texture (QSGTexture::removedFromAtlas()) would fail. The next commit makes sure that ::removedFromAtlas() works fine with QSGTextureView, too, even though currently it is not used (I used a trick to disable layering instead of using QSGTextureView in music artist header).

Request review @chub.

Merge request reports

Loading