1273804 - mask-position is not animatable

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

Check what mstange asked in bug 1272970 comment 3, then found this regression.

mask-size and mask-position are both animatable before introducing nsChangeHint_UpdateBackgroundPosition.

Comment 1

[u459114]

Short term solution:
With the change in bug 1258286, update different hint base on layer type in nsStyleImageLayers::Layer::CalcDifference

Long term solution:
Bug 1234485, create a animated mask layer. Not totally be the same with bug bug 550426, but some logic can be shared. 

[1] https://hg.mozilla.org/mozilla-central/rev/e723928647d4

Comment 2

[u459114]

Attachment: Bug 1273804 - Use nsChangeHint_RepaintFrame hint for position change of a mask layer

Review commit: https://reviewboard.mozilla.org/r/56896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/56896/

Comment 3

[u459114]

hi heycam,
this patch roll back behavior of mask animation before bug 550426 landed. in long term, we need bug 1234485 fixed to have smoother mask-position change animation

Comment 4

[Cameron McCormack (:heycam)]

Comment on attachment 8758672 [details]
Bug 1273804 - Use nsChangeHint_RepaintFrame hint for position change of a mask layer

https://reviewboard.mozilla.org/r/56896/#review56204

Do you have a test that fails without this patch?  If so, that would be good to check in here too.

::: layout/style/nsStyleStruct.h:683
(Diff revision 1)
>      // whose root <svg> node has a viewBox.
>      bool RenderingMightDependOnPositioningAreaSizeChange() const;
>  
>      // Compute the change hint required by changes in just this layer.
> -    nsChangeHint CalcDifference(const Layer& aOther) const;
> +    nsChangeHint CalcDifference(const Layer& aOther,
> +                                nsChangeHint aPositionChangeHint) const;

Please add a comment that describes what aPositionChangeHint is used for.

Comment 5

[u459114]

https://reviewboard.mozilla.org/r/56896/#review56204

Astley will create some in Bug 1273807 seperately.

Comment 6

[u459114]

Comment on attachment 8758672 [details]
Bug 1273804 - Use nsChangeHint_RepaintFrame hint for position change of a mask layer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56896/diff/1-2/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e6e22da34de
Use nsChangeHint_RepaintFrame hint for position change of a mask layer r=heycam

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6e6e22da34de

Comment 9

[Jean-Yves Perrier [:teoli]]

I have a question here, I think this landed in the 50 cycle, so the target milestone should be Mozilla49 instead of Mozilla50. Is this correct?

Comment 10

[Carsten Book [:Tomcat]]

(In reply to Jean-Yves Perrier [:teoli] from comment #9)
> I have a question here, I think this landed in the 50 cycle, so the target
> milestone should be Mozilla49 instead of Mozilla50. Is this correct?

hi,

yeah you are right, resetting this. thanks!

Comment 11

[Jean-Yves Perrier [:teoli]]

Given it is only available in Nightly for the moment, I only made a comment there: https://developer.mozilla.org/en-US/Firefox/Releases/50#CSS
(the mask-position page indicates that the property is animatable)