qt: support background coloring and fix hard edge in round image
I'm not sure what is better, fwidth()
may not be so cheap, so providing the soft edge as a uniform also makes sense (current approach is providing 1.0 / Math.min(width, height)
as uniform).
Currently, we are not making use of custom soft edge as an effect, but rather have such a soft edge to merely act as antialiasing.
It seems that fwidth()
would be more accurate to use for the anti aliasing here. It is hard for me to observe the difference, so I would be fine keeping the current approach.
In https://blog.frost.kiwi/analytical-anti-aliasing/#implementation, the difference can be seen ("Pixel size method" being fwidth
(Screen-space derivative) or Precalculate pixel size
).
The problem is more about having a hard edge currently, because without shrinking the outer soft edge exceeds the rectangle boundaries. This is also explained in https://blog.frost.kiwi/analytical-anti-aliasing/#unmentioned-challenges.
Request review @chub.
Merge request reports
Activity
added MRStatus::Reviewable label
- Resolved by Fatih Uzunoğlu
I should probably shrink the shadow as well...
added 4 commits
- cb136e5e - qt: use `fwidth()` for determining the default soft edge boundaries in `SDFAARoundedTexture.frag`
- cff8e7ce - qt: shrink the distance to provide breathing room in `SDFAARoundedTexture.frag`
- 7bbdea0b - qml: make painted size not cover the anti aliased border margins
- 1b423679 - qml: use painted width of the target image in MusicArtistDelegate shadow
Toggle commit list- Resolved by Fatih Uzunoğlu
It seems that
fwidth()
would be more accurate to use for the anti aliasing here. It is hard for me to observe the difference, so I would be fine keeping the current approach.It should be noted that since we need to shrink the shadow and other elements (as we don't want them to start with after the anti aliased borders), we need to know how much the texture is shrunk. With
fwidth()
that does not seem possible, so here I assumed that soft edge max (even though it is not used in favor offwidth()
)1.0 / Math.min(width, height)
is not too different thanfwidth()
. If we can not have such assumption, we can not usefwidth()
.
added 179 commits
-
1b423679...687f2080 - 173 commits from branch
videolan:master
- bd47c4a9 - qt: support background coloring in `SDFAARoundedTexture.frag`
- 94677fa6 - qml: support background coloring in `RoundImage.qml`
- e323d017 - qt: add support for using `fwidth()` in `SDFAARoundedTexture.frag`
- 0fdc8763 - qt: shrink the distance to provide breathing room in `SDFAARoundedTexture.frag`
- 44f4a05d - qml: make painted size not cover the anti aliased border margins
- bd59dd11 - qml: use painted width of the target image in MusicArtistDelegate shadow
Toggle commit list-
1b423679...687f2080 - 173 commits from branch
added Component::Interface: Qt label
changed milestone to %4.0
added MRStatus::Acceptable label and removed MRStatus::Reviewable label
As a side note, the article
shrinksgrows the quad with the vertex shader (and shrinks with the fragment shader) which makes sense. However here we can not do that as if we use custom vertex shader, the shader effect nodes wouldn't be batched together.For the 2D case, we could implement a kind of NV_conservative_raster_dilate ourselves, by growing the quad in the vertex shader by half a pixel and shrinking the signed distance field by half a pixel in the fragment shader. And this is exactly what’s happening in the 2D demos on this page!
However, this has less relevance when
softEdgeMax
is used as effect (large value) and not only for antialiasing, as I'm not sure if it is a good idea to grow the quad that much (without the Quick item knowing that).MultiEffect
also reportedly grows, but it is done in the item level (and not in vertex shader). Withpadding
here, we do pretty much the same. But why not grow, then? I tried to explain that in the other comment:I believe shrinkage is better, because then clipping would not cause issues, and other Qt Quick elements also shrink (such as
border
of Rectangle, orImage
antialiasing).Edited by Fatih Uzunoğlu- Resolved by Fatih Uzunoğlu
added 6 commits
- 6aebe8b7 - qml: provide padding property that represents the shrinkage in `RoundImage`
- 52f92c9a - qml: use painted width of the target image in MusicArtistDelegate shadow
- ea227eda - qml: shrink shadows if target image is also shrunk
- 4b2f24c7 - qml: set implicit size rather than final size in `NetworkGridItem` overlay
- ecbea83a - qml: set implicit size rather than final size in `VideoGridItem` overlay
- 3c80d553 - qml: use painted size for image overlay in `MediaCover`
Toggle commit listadded 6 commits
- d86427c0 - qml: provide padding property that represents the shrinkage in `RoundImage`
- 60fe86cc - qml: use painted width of the target image in MusicArtistDelegate shadow
- b4a30e7a - qml: shrink shadows if target image is also shrunk
- 95bd4f1e - qml: set implicit size rather than final size in `NetworkGridItem` overlay
- c49c4441 - qml: set implicit size rather than final size in `VideoGridItem` overlay
- 82fb7d05 - qml: use painted size for image overlay in `MediaCover`
Toggle commit listadded 6 commits
- d9e6d233 - qml: provide padding property that represents the shrinkage in `RoundImage`
- 7939b6b1 - qml: use painted width of the target image in MusicArtistDelegate shadow
- de7b4870 - qml: shrink shadows if target image is also shrunk
- 798c506b - qml: set implicit size rather than final size in `NetworkGridItem` overlay
- adcbeb38 - qml: set implicit size rather than final size in `VideoGridItem` overlay
- f9149683 - qml: use painted size for image overlay in `MediaCover`
Toggle commit listadded 6 commits
- 631a45f2 - qml: provide padding property that represents the shrinkage in `RoundImage`
- 9f9dee6b - qml: use painted width of the target image in MusicArtistDelegate shadow
- 43a31c7a - qml: shrink shadows if target image is also shrunk
- 416ace60 - qml: set implicit size rather than final size in `NetworkGridItem` overlay
- 70dfd1ed - qml: set implicit size rather than final size in `VideoGridItem` overlay
- bbaf5988 - qml: use painted size for image overlay in `MediaCover`
Toggle commit listI realized these:
- Considering shrinkage in
paintedWidth/Height
may be confusing. The main reason is thatImage
does not seem to do that when it uses antialiasing, so I also stopped considering shrinkage in painted size. Instead, I added a readonlypadding
property that tells the shrinkage.padding
is similar toMultiEffect
'spaddingRect
and itsautoPaddingEnabled
functionality (https://doc.qt.io/qt-6/qml-qtquick-effects-multieffect.html#autoPaddingEnabled-prop). However, instead of shrinkingMultiEffect
reportedly grows. I believe shrinkage is better, because then clipping would not cause issues, and other Qt Quick elements also shrink (such asborder
of Rectangle, orImage
antialiasing). Currentlypadding
only considers the outer antialiasing border (softEdgeMax
), but in the future we can add functionality to have extra padding if necessary. - Unlike shadows, overlay items should not shrink as if the antialiased border of the exceeds the overlay items (such as the progress bar) due to different curvature of the corners, or their antialiased borders overlap where they blend in to a different color, it does not seem to look good. So I no longer shrink the overlay items.
No major changes.
Unlike shadows, overlay items should not shrink as it does not seem to look good. So I no longer shrink the overlay items.
For example, this:
Looks more natural than this:
- Considering shrinkage in
If someone wants to clearly see how the shadows shrink,
softEdgeMax
can be set something large such as0.5
.Here are screenshots when
softEdgeMax
is0.5
with shadows shrinking with the image:Without shrinkage:
Although the default
softEdgeMax
is used for antialiasing, so it is really small. Currently, it is1. / Math.min(width, height)
(not very much different thanfwidth()
). So with that it is harder to notice if shadows shrink or not alongside the image (this screenshots is exaggeration).I think shadows really looks well with shrinkage (unlike overlay items).
added MRStatus::Accepted label and removed MRStatus::Acceptable label
MR Acceptance result
This MergeRequest has been Accepted! Congratulations.MR acceptance checks details:
-
MR should be considered mergeable by Gitlab -
Last pipeline should be successful -
MergeRequest should have at least one external review and/or vote -
All threads should be resolved, have votes and score > 0 -
MergeRequest should have no activity (threads/votes) for (72h/72h)
-
added 109 commits
-
bbaf5988...d9076332 - 99 commits from branch
videolan:master
- a2a0afbf - qt: support background coloring in `SDFAARoundedTexture.frag`
- a8d53a84 - qml: support background coloring in `RoundImage.qml`
- 189c4284 - qt: add support for using `fwidth()` in `SDFAARoundedTexture.frag`
- 295980e2 - qt: shrink the distance to provide breathing room in `SDFAARoundedTexture.frag`
- d8b771a7 - qml: provide padding property that represents the shrinkage in `RoundImage`
- f0ff7c47 - qml: use painted width of the target image in MusicArtistDelegate shadow
- 1d05fd47 - qml: shrink shadows if target image is also shrunk
- 56fab50a - qml: set implicit size rather than final size in `NetworkGridItem` overlay
- 06fa8c28 - qml: set implicit size rather than final size in `VideoGridItem` overlay
- 88981f67 - qml: use painted size for image overlay in `MediaCover`
Toggle commit list-
bbaf5988...d9076332 - 99 commits from branch
enabled an automatic merge when all merge checks for 88981f67 pass
mentioned in merge request !6858