1297944 - LayerManagerComposite::SetShadowOpacity should involve a flag which indicates whether the opacity is specified by animation or not

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

In bug 1279071, SetShadowOpacitySetByAnimation() which has an argument of the flag has been introduced, but we could unify SetShadowOpacity() and SetShadowOpacitySetByAnimation().

Assigning to me temporary just in case that this bug does not bother gfx team triage.

Comment 1

[Yuri Kazuna]

Attachment: 1297944.patch Unified SetShadowOpacitySetByAnimation(bool) into SetShadowOpacity(float, bool).

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8785547 [details] [diff] [review]
1297944.patch
Unified SetShadowOpacitySetByAnimation(bool) into SetShadowOpacity(float, bool).

This code is good but I prefer to use enum value instead of boolean.

Could you please add an enum class and use it something like this;

enum class ByWhich {
  Animation,
  NonAnimation
}

I don't think it's a great name but I have no idea for now.

Comment 3

[Yuri Kazuna]

It seems that SetShadowTransformSetByAnimation(bool) exists besides SetShadowOpacitySetByAnimation(bool).
Should the name of the enum class adjust these?

Comment 4

[Yuri Kazuna]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)
> Comment on attachment 8785547 [details] [diff] [review]
> 1297944.patch
> Unified SetShadowOpacitySetByAnimation(bool) into SetShadowOpacity(float,
> bool).
> 
> This code is good but I prefer to use enum value instead of boolean.
> 
> Could you please add an enum class and use it something like this;
> 
> enum class ByWhich {
>   Animation,
>   NonAnimation
> }
> 
> I don't think it's a great name but I have no idea for now.

It seems that SetShadowTransformSetByAnimation(bool) exists besides SetShadowOpacitySetByAnimation(bool).
Should the name of the enum class adjust these?

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Please leave the function for transform as it is in this bug.  There are many callers of SetShadowTransform().

Comment 6

[Darkspirit]

bug 1727672