D28625: Use ShadowedRectangle for Card backgrounds

[Carson Black]

cblack added a comment.


  This seems to be causing regressions to card behaviour for me.
  
  Cards are running together when they weren't previously:
  F8252161: ksnip_20200421-162339.png 
  
  The images are:
  
  1. having non-deterministic texture issues (amdgpu/Mesa)
  2. getting distorted when they previously weren't
  
  F8252164: ksnip_20200421-162514.png 
  
  Additionally, this appears to be having a non-trivial performance regression 
to large amounts of cards, as Ikona will freeze for much longer with this patch 
in its Icon Browser than without cards.
  
  Additionally, console output spams something like this when cards are 
involved:
  
/usr/lib64/qml/org/kde/kirigami.2/AbstractCard.qml:31: TypeError: Type 
error (file:usr/lib64/qml/org/kde/kirigami.2/AbstractCard.qml:31, )

REPOSITORY
  R169 Kirigami

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

To: ahiemstra, #kirigami, #vdg, cblack, mart
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson

D28625: Use ShadowedRectangle for Card backgrounds

[Arjen Hiemstra]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:e770b0ddd2af: Use ShadowedRectangle for Card backgrounds 
(authored by ahiemstra).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28625?vs=80735=80736

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

AFFECTED FILES
  src/colorutils.cpp
  src/colorutils.h
  src/controls/AbstractCard.qml
  src/controls/Card.qml
  src/controls/ShadowedImage.qml
  src/controls/private/BannerImage.qml
  src/controls/private/DefaultCardBackground.qml
  src/scenegraph/shaders/sdf.glsl
  src/scenegraph/shadowedtexturenode.cpp
  src/shadowedtexture.cpp

To: ahiemstra, #kirigami, #vdg, cblack, mart
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson

D28625: Use ShadowedRectangle for Card backgrounds

[Arjen Hiemstra]

ahiemstra updated this revision to Diff 80735.
ahiemstra added a comment.


  - Rebase on most recent master
  - Use ShadowedRectangleNode for ShadowedTexture if source is not set
  - Use transparent color for BannerImage's ShadowedImage
  - Hide BannerImage scrim if source or title is not set
  - Don't set ShadowedTexture source if Image is not properly loaded
  - Add tintWithAlpha method to ColorUtils
  - Use new tintWithAlpha for border colors

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28625?vs=79491=80735

BRANCH
  card_shadowrect

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

AFFECTED FILES
  src/colorutils.cpp
  src/colorutils.h
  src/controls/AbstractCard.qml
  src/controls/Card.qml
  src/controls/ShadowedImage.qml
  src/controls/private/BannerImage.qml
  src/controls/private/DefaultCardBackground.qml
  src/scenegraph/shaders/sdf.glsl
  src/scenegraph/shadowedtexturenode.cpp
  src/shadowedtexture.cpp

To: ahiemstra, #kirigami, #vdg, cblack, mart
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson

D28625: Use ShadowedRectangle for Card backgrounds

[Arjen Hiemstra]

ahiemstra added a comment.


  In D28625#652529 , @mart wrote:
  
  > In D28625#642954 , @cblack wrote:
  >
  > > Looks like a nice visual improvement.
  > >
  > > Are the changes to the scenegraph stuff related? They don't look like a 
related change to making the Kirigami.Card use the ShadowedRectangle to me.
  >
  >
  > they should be, as far i know, a fix for the 1 pixel border that had some 
blurriness.
  >  @ahiemstra can you confirm?
  
  
  They're both things I encountered while implementing this. One is a crash 
fix, the other is indeed a fix for pixel correctness.

INLINE COMMENTS

> mart wrote in AbstractCard.qml:31
> as we have a ColorUtils class now, since i've seen this super long thing just 
> to make a color translucent many things, is maybe worth to have a 
> ColorUtils.opacity(Kirigami.Theme.highlightColor, 0.3) instead?

This basically does the same thing as Kirigami.Separator. I think it'd be good 
to have this entire thing in ColorUtils yeah.

REPOSITORY
  R169 Kirigami

BRANCH
  card_shadowrect

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

To: ahiemstra, #kirigami, #vdg, cblack, mart
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson

D28625: Use ShadowedRectangle for Card backgrounds

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> AbstractCard.qml:31
> +id: bg
> +readonly property color pressedColor: 
> Qt.tint(Kirigami.Theme.backgroundColor, Qt.rgba(
> +
> Kirigami.Theme.highlightColor.r,

as we have a ColorUtils class now, since i've seen this super long thing just 
to make a color translucent many things, is maybe worth to have a 
ColorUtils.opacity(Kirigami.Theme.highlightColor, 0.3) instead?

REPOSITORY
  R169 Kirigami

BRANCH
  card_shadowrect

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

To: ahiemstra, #kirigami, #vdg, cblack
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson

D28625: Use ShadowedRectangle for Card backgrounds

[Marco Martin]

mart added a comment.


  In D28625#642954 , @cblack wrote:
  
  > Looks like a nice visual improvement.
  >
  > Are the changes to the scenegraph stuff related? They don't look like a 
related change to making the Kirigami.Card use the ShadowedRectangle to me.
  
  
  they should be, as far i know, a fix for the 1 pixel border that had some 
blurriness.
  @ahiemstra can you confirm?

REPOSITORY
  R169 Kirigami

BRANCH
  card_shadowrect

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

To: ahiemstra, #kirigami, #vdg, cblack
Cc: mart, ngraham, nicolasfella, cblack, plasma-devel, fbampaloukas, GB_2, 
domson, dkardarakos, apol, ahiemstra, davidedmundson