u459114 Assignee
	Description • 8 years ago

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.

	u459114 Assignee
	Comment 1 • 8 years ago

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

	u459114 Assignee
	Comment 2 • 8 years ago

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

	u459114 Assignee
	Comment 3 • 8 years ago

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

	Cameron McCormack (:heycam)
	Comment 4 • 8 years ago

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.

	u459114 Assignee
	Comment 5 • 8 years ago

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

Astley will create some in Bug 1273807 seperately.

	u459114 Assignee
	Comment 6 • 8 years ago

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/

	Pulsebot
	Comment 7 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 8 • 8 years ago
bugherder

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

	Jean-Yves Perrier [:teoli]
	Comment 9 • 8 years ago

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?

	Carsten Book [:Tomcat]
	Comment 10 • 8 years ago

(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!

	Jean-Yves Perrier [:teoli]
	Comment 11 • 8 years ago

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)