kossebau added a comment.

  With cross-casting I meant the type of cast where the types involved are not 
in same inheritance branch

INLINE COMMENTS

> davidre wrote in breezebutton.cpp:148
> I don't think it can fail. decoration() is a QPointer 
> (https://api.kde.org/kdecoration/html/classKDecoration2_1_1DecorationButton.html)
>  do you mean that with cross-cast?

Looking at the rest of the code, it always tests for decoration() resolving to 
an existing  `Decoration` pointer. 
While I would have expected it might be ensured that decoration always is of 
type `Breeze::Decoration`, but `Button::create(...)` takes a 
`KDecoration2::Decoration`, so at least by code in this class this is not 
ensured.

Now, `Button::paint(...)` already has half the check ealier, by the

  if (!decoration()) return;

But this does not ensure we have a `Breeze::Decoration` type.

And QPointer only ensures that the pointer is updated if the object is deleted 
elsewhere. It still can be nullptr, and possibly this is done here as the 
decoration object is not owned by us, so something might delete it behind the 
button's back.

So IMHO this logic might need another check by someone who can tell if the 
right type can be still assumed (which ideally then should be reflected in the 
API, so it is clear no other type could be present.

> breezebutton.cpp:151
>              decoration()->client().data()->icon().paint(painter, 
> iconRect.toRect());
> -
> +            KIconLoader::global()->resetPalette();
>  

Assumes there is no other custom palette set before we set ours here. Does that 
assumption hold? Or could KWin & Co do mess with that palette as well?

REPOSITORY
  R31 Breeze

BRANCH
  icons (branched from master)

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

To: davidre, #plasma, #vdg, ngraham
Cc: kossebau, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to