Closed Bug 1370034 Opened 7 years ago Closed 7 years ago

Implement new CSS properties -moz-window-opacity, -moz-window-transform and -moz-window-transform-origin on Mac for efficient panel animations

Categories

(Core :: CSS Parsing and Computation, enhancement)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
All
macOS
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(7 files)

Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).
59 bytes, text/x-review-board-request
dbaron
: review+
Details
Bug 1370034 - Add nsIWidget::SetWindowOpacity and implement it on Mac.
59 bytes, text/x-review-board-request
spohl
: review+
Details
Bug 1370034 - Implement UpdateWidgetProperties for top level windows and for popups, and call nsIWidget::SetWindowShadow.
59 bytes, text/x-review-board-request
dbaron
: review+
Details
Bug 1370034 - Add CSS properties -moz-window-transform and -moz-window-transform-origin, style system parts (excluding stylo).
59 bytes, text/x-review-board-request
dbaron
: review+
Details
Bug 1370034 - Add nsIWidget::SetWindowTransform and implement it on Mac.
59 bytes, text/x-review-board-request
spohl
: review+
Details
Bug 1370034 - Call SetWindowTransform with the right values from -moz-window-transform(-origin).
59 bytes, text/x-review-board-request
dbaron
: review+
Details
Bug 1370034 - Allow disabling window transforms using a pref.
59 bytes, text/x-review-board-request
spohl
: review+
Details 
Our panel animations currently apply an opacity and a transform to the content that is painted inside of the panel. This has bad effects on the window shadow on Mac: The system computes the shadow's shape based on the pixels that are drawn into the window, so we need to ask it to recompute the shadow on every animation frame.

It would be better if we could instead change the opacity and the transform of the window itself. Those would apply to the whole window including the shadow, and they would apply *outside* the pixels inside the window, so the window's "shape" would stay fixed during the animation.

macOS has a public API to change a window's opacity: -[NSWindow setOpacity:].
It also has a private, but widely-used API to change a window's transform: CGSSetWindowTransform.

Instead of re-purposing the existing CSS properties "transform" and "opacity" when used on panels, I'd prefer to add new CSS properties. That's going to make the layout part of this change a lot simpler, because we won't have to add tons of special cases.

I'm calling these properties -moz-window-opacity, -moz-window-transform, and -moz-window-transform-origin. They can be animated using CSS transitions.
Blocks: 1291457
I tried to make these properties CSS_PROPERTY_ENABLED_IN_UA_SHEETS_AND_CHROME but that broke CSS transitions on them. Is that a known issue?

These patches don't implement the new properties for stylo. I'd appreciate help with that.
Flags: needinfo?(xidorn+moz)
I wasn't aware of that issue before, and I'm not familiar with transition stuff so not sure what's happening there. Redirect the question to birtles.
Flags: needinfo?(xidorn+moz) → needinfo?(bbirtles)
If it's not known, then I may have made a mistake. I still have some test failures on these patches. Once they are fixed, I'll try CSS_PROPERTY_ENABLED_IN_UA_SHEETS_AND_CHROME again.
Flags: needinfo?(bbirtles)
That may simply be because we never tried animating internal-only properties before. This mechanism was added not very long ago, so I wouldn't be surprised if it is broken in some cases.
Yeah, there are a bunch of places where we pass CSSEnabledState::eForAllContent unconditionally:

  http://searchfox.org/mozilla-central/search?q=CSSEnabledState%3A%3AeForAllContent&case=false&regexp=false&path=dom%2Fanimation

Like Xidorn said, this is a fairly new mechanism and we've probably just never tried animating internal-only properties before.
I'd also note that this patch doesn't do what this comment says:
https://searchfox.org/mozilla-central/source/layout/style/nsCSSProps.h#265-268
i.e., wrapping in CSS_PROP_LIST_EXCLUDE_INTERNAL.  I'm honestly a little rusty on the distinctions between these, although I suspect that you may want to use CSS_PROPERTY_ENABLED_IN_UA_SHEETS_AND_CHROME instead (and make animations on it work).
Flags: needinfo?(mstange)
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #28)
> I'd also note that this patch doesn't do what this comment says:
> https://searchfox.org/mozilla-central/source/layout/style/nsCSSProps.h#265-
> 268
> i.e., wrapping in CSS_PROP_LIST_EXCLUDE_INTERNAL.

It does; it puts the properties into the #ifdef CSS_PROP_LIST_EXCLUDE_INTERNAL block that's already there for -moz-window-shadow.
Flags: needinfo?(mstange)
Also, I think using CSS_PROPERTY_INTERNAL flag without the ifdefs doesn't actually prevent exposing the property to the Web.  We should probably have assertions verifying that we don't have any properties set up that way.
OK, if it's actually properly internal -- then how does the test you added to property_database.js pass?
Flags: needinfo?(mstange)
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #30)
> Also, I think using CSS_PROPERTY_INTERNAL flag without the ifdefs doesn't
> actually prevent exposing the property to the Web.  We should probably have
> assertions verifying that we don't have any properties set up that way.

We have such check: https://dxr.mozilla.org/mozilla-central/rev/a49112c7a5765802096b3fc298069b9495436107/layout/style/nsCSSProps.cpp#33-46
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #31)
> OK, if it's actually properly internal -- then how does the test you added
> to property_database.js pass?

That "test" is inside an "if (false)" block and doesn't actually run. Sorry, I just followed what was done in the context in that file. I can remove the test additions if that would be less misleading.
Flags: needinfo?(mstange)
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #32)
> (In reply to David Baron :dbaron: ⌚️UTC-7 from comment #30)
> > Also, I think using CSS_PROPERTY_INTERNAL flag without the ifdefs doesn't
> > actually prevent exposing the property to the Web.  We should probably have
> > assertions verifying that we don't have any properties set up that way.
> 
> We have such check:
> https://dxr.mozilla.org/mozilla-central/rev/
> a49112c7a5765802096b3fc298069b9495436107/layout/style/nsCSSProps.cpp#33-46

Yes, and I can confirm that it's working, because I actually ran into that check at some point during development.
(In reply to Brian Birtles (:birtles) from comment #20)
> Yeah, there are a bunch of places where we pass
> CSSEnabledState::eForAllContent unconditionally:
> 
> http://searchfox.org/mozilla-central/search?q=CSSEnabledState%3A%3AeForAllContent&case=false&regexp=false&path=dom%2Fanimation

Experimentally, I replaced eForAllContent with eIgnoreEnabledState in a few places. In addition to the ones in KeyframeUtils.cpp, I also had to change these two:
http://searchfox.org/mozilla-central/rev/12afa8cc9cea4a141494f767541b411845eef7a0/layout/style/nsRuleNode.cpp#5614
http://searchfox.org/mozilla-central/rev/12afa8cc9cea4a141494f767541b411845eef7a0/layout/style/nsTransitionManager.cpp#862

After that, the transitions started working.

If we want to fix this, I'll need some guidance on where to obtain the right enabled state for the various cases. Alternatively, maybe somebody else who already knows the code could fix this?
Or we could go ahead with the current patches and use only CSS_PROPERTY_INTERNAL and not CSS_PROPERTY_ENABLED_IN_UA_SHEETS_AND_CHROME.
Comment on attachment 8874168 [details]
Bug 1370034 - Add nsIWidget::SetWindowOpacity and implement it on Mac.

https://reviewboard.mozilla.org/r/145588/#review152484

::: widget/cocoa/nsCocoaWindow.mm:2172
(Diff revision 4)
> +void
> +nsCocoaWindow::SetWindowOpacity(float aOpacity)
> +{
> +  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
> +
> +  if (!mWindow)

nit: Use {}
Attachment #8874168 - Flags: review?(spohl.mozilla.bugs) → review+
Comment on attachment 8874171 [details]
Bug 1370034 - Add nsIWidget::SetWindowTransform and implement it on Mac.

https://reviewboard.mozilla.org/r/145594/#review152492

::: widget/cocoa/nsCocoaWindow.mm:2182
(Diff revision 7)
>  
>    NS_OBJC_END_TRY_ABORT_BLOCK;
>  }
>  
> +static inline CGAffineTransform
> +GfxMatrixToCGAffineTransform(const gfx::Matrix &m)

nit: & after gfx::Matrix instead of before m

::: widget/cocoa/nsCocoaWindow.mm:2199
(Diff revision 7)
> +void
> +nsCocoaWindow::SetWindowTransform(const gfx::Matrix& aTransform)
> +{
> +  NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
> +
> +  if (!mWindow)

nit: Use {}
Attachment #8874171 - Flags: review?(spohl.mozilla.bugs) → review+
Comment on attachment 8875935 [details]
Bug 1370034 - Allow disabling window transforms using a pref.

https://reviewboard.mozilla.org/r/147334/#review152498
Attachment #8875935 - Flags: review?(spohl.mozilla.bugs) → review+
Emilio told me that the stylo changes don't need to block the initial landing of this bug. We also don't seem to be using stylo for chrome properties yet.
Comment on attachment 8874167 [details]
Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145586/#review154598

r=dbaron if you've done the following and tests pass:

::: layout/style/test/property_database.js:7908
(Diff revision 5)
> +  gCSSProperties["-moz-window-opacity"] = {
> +    // domProp: "MozWindowOpacity",
> +    inherited: false,
> +    type: CSS_TYPE_LONGHAND,
> +    initial_values: [ "1", "17", "397.376", "3e1", "3e+1", "3e0", "3e+0", "3e-0" ],
> +    other_values: [ "0", "0.4", "0.0000", "-3", "3e-1" ],
> +    invalid_values: [ "0px", "1px", "20%", "default", "auto" ]
> +  };
> +

Please do a try run with this *outside* the if-false and with the property not marked as internal in nsCSSPropList.h, and make sure all the mochitests in layout/style/test/ pass in that configuration.
Attachment #8874167 - Flags: review?(dbaron) → review+
Comment on attachment 8874169 [details]
Bug 1370034 - Implement UpdateWidgetProperties for top level windows and for popups, and call nsIWidget::SetWindowShadow.

https://reviewboard.mozilla.org/r/145590/#review154600

::: layout/generic/nsFrame.cpp:10481
(Diff revision 5)
>  nsIFrame::IsScrolledOutOfView()
>  {
>    return IsFrameScrolledOutOfView(this);
>  }
>  
> +static nsIWidget*

This should return already_AddRefed<nsIWidget>, and the last line should be "return mainWidget.forget()".  Otherwise you're dropping the reference you acquired and addrefing it again in the caller -- which is both extra refcount traffic and dangerous.

::: layout/generic/nsFrame.cpp:10481
(Diff revision 5)
> +static nsIWidget*
> +GetWindowWidget(nsPresContext* aPresContext)
> +{
> +  nsCOMPtr<nsISupports> container = aPresContext->Document()->GetContainer();
> +  nsCOMPtr<nsIBaseWindow> baseWindow = do_QueryInterface(container);
> +  if (!baseWindow)
> +    return nullptr;
> +
> +  nsCOMPtr<nsIWidget> mainWidget;
> +  baseWindow->GetMainWidget(getter_AddRefs(mainWidget));
> +  return mainWidget;
> +}

Please add comments explaining why you need this function rather than one of the existing functions nsPresContext::GetRootWidget or nsPresContext::GetNearestWidget or nsIFrame::GetNearestWidget, or use one of them.  (My assumption is it's because you want the *top* widget.)

That said, would it be simpler to use aPresContext->GetRootWidget()->GetTopLevelWidget()?

::: layout/generic/nsFrame.cpp:10498
(Diff revision 5)
> +
> +void
> +nsIFrame::UpdateWidgetProperties()
> +{
> +  nsPresContext* presContext = PresContext();
> +  if (presContext->GetParentPresContext() || !presContext->IsChrome()) {

Please use nsPresContext::IsRoot() rather than the somewhat-expensive  nsPresContext::GetParentPresContext().

::: layout/generic/nsFrame.cpp:10503
(Diff revision 5)
> +  if (presContext->GetParentPresContext() || !presContext->IsChrome()) {
> +    // Don't do anything for documents that aren't the root chrome document.
> +    return;
> +  }
> +  nsIFrame* rootFrame =
> +    presContext->PresShell()->FrameConstructor()->GetRootElementStyleFrame();

please use presContext->FrameConstructor() rather than presContext->PresShell()->FrameConstructor()
Attachment #8874169 - Flags: review?(dbaron) → review+
Comment on attachment 8874167 [details]
Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145586/#review154598

> Please do a try run with this *outside* the if-false and with the property not marked as internal in nsCSSPropList.h, and make sure all the mochitests in layout/style/test/ pass in that configuration.

I've started a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd4b01a8541ee03a0a801a2de46e633fb76a97dd
(In reply to Markus Stange [:mstange] from comment #62)
> Comment on attachment 8874167 [details]
> Bug 1370034 - Add -moz-window-opacity property, style system parts
> (excluding stylo).
> 
> https://reviewboard.mozilla.org/r/145586/#review154598
> 
> > Please do a try run with this *outside* the if-false and with the property not marked as internal in nsCSSPropList.h, and make sure all the mochitests in layout/style/test/ pass in that configuration.
> 
> I've started a try push at
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd4b01a8541ee03a0a801a2de46e633fb76a97dd

Whoops, forgot to re-enable the "domProp: ..." lines. Trying again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=82412b33ae6da5bc5b495179535de80756772739
Comment on attachment 8874170 [details]
Bug 1370034 - Add CSS properties -moz-window-transform and -moz-window-transform-origin, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145592/#review154708

::: devtools/shared/css/generated/properties-db.js:1518
(Diff revision 7)
> +  "-moz-window-transform": {
> +    "isInherited": false,

I'm presuming you generated these changes with "./mach devtools-css-db".  If not, you should have done it that way.  I'm not reviewing them for substance.  (Same for previous patch.)

::: layout/style/nsCSSParser.cpp:16239
(Diff revision 7)
> -CSSParserImpl::ParseTransform(bool aIsPrefixed, bool aDisallowRelativeValues)
> +CSSParserImpl::ParseTransform(bool aIsPrefixed, nsCSSPropertyID aProperty,
> +                              bool aDisallowRelativeValues)

Do you intend to accept 3-D transform functions here?  If so, what are you doing with them?

::: layout/style/nsCSSPropList.h:4507
(Diff revision 7)
>      "",
>      VARIANT_HN,
>      nullptr,
>      offsetof(nsStyleUIReset, mWindowOpacity),
>      eStyleAnimType_float)
> +CSS_PROP_UIRESET(

You should do the same testing drill for these changes -- run mochitests with them not marked as internal.

::: layout/style/nsRuleNode.cpp:1745
(Diff revision 7)
>    NS_NOTREACHED("SetFactor: inappropriate unit");
>  }
>  
> +static void
> +SetTransformValue(const nsCSSValue& aValue, RefPtr<nsCSSValueSharedList>& aField,
> +                  RuleNodeCacheConditions& aConditions, nsCSSValueSharedList* aParentValue)

Please make aParentValue nsCSSValueSharedList* const, and then make sure the whole thing wraps at less than 80 lines.

::: layout/style/nsStyleStruct.cpp:4373
(Diff revision 7)
>  nsStyleUIReset::CalcDifference(const nsStyleUIReset& aNewData) const
>  {
>    // ignore mIMEMode
>    if (mForceBrokenImageIcon != aNewData.mForceBrokenImageIcon) {
>      return nsChangeHint_ReconstructFrame;
>    }
>    if (mWindowShadow != aNewData.mWindowShadow) {
>      // We really need just an nsChangeHint_SyncFrameView, except
>      // on an ancestor of the frame, so we get that by doing a
>      // reflow.
>      return NS_STYLE_HINT_REFLOW;
>    }
>    if (mUserSelect != aNewData.mUserSelect) {
>      return NS_STYLE_HINT_VISUAL;
>    }
>  
>    if (mWindowDragging != aNewData.mWindowDragging) {
>      return nsChangeHint_SchedulePaint;
>    }
>  
>    if (mWindowOpacity != aNewData.mWindowOpacity) {
>      return nsChangeHint_UpdateWidgetProperties;
>    }
>  
> +  if (!mSpecifiedWindowTransform != !aNewData.mSpecifiedWindowTransform ||
> +      (mSpecifiedWindowTransform &&
> +       *mSpecifiedWindowTransform != *aNewData.mSpecifiedWindowTransform)) {
> +    return nsChangeHint_UpdateWidgetProperties;
> +  }
> +
> +  for (uint8_t index = 0; index < 2; ++index) {
> +    if (mWindowTransformOrigin[index] != aNewData.mWindowTransformOrigin[index]) {
> +      return nsChangeHint_UpdateWidgetProperties;
> +    }
> +  }
> +
>    if (mIMEMode != aNewData.mIMEMode) {
>      return nsChangeHint_NeutralChange;

This method needs restructuring.  I should have caught this in the previous patch.  You need to do the right thing when multiple properties change, which this doesn't do.  You need to be able to send UpdateWidgetProperties hints along with others.

(Or is UpdateWidgetProperties somehow implied by SchedulePaint?)
Attachment #8874170 - Flags: review?(dbaron) → review+
Comment on attachment 8874172 [details]
Bug 1370034 - Call SetWindowTransform with the right values from -moz-window-transform(-origin).

https://reviewboard.mozilla.org/r/145596/#review154716

::: layout/generic/nsFrame.cpp:10505
(Diff revision 9)
> +  // Compute the -moz-window-transform-origin value in device pixels.
> +  float transformOrigin[2];
> +  nsStyleTransformMatrix::TransformReferenceBox::DimensionGetter dimensionGetter[] =
> +    { &nsStyleTransformMatrix::TransformReferenceBox::Width,
> +      &nsStyleTransformMatrix::TransformReferenceBox::Height };
> +  for (uint8_t index = 0; index < 2; ++index) {
> +    const nsStyleCoord& originValue  = uiReset->mWindowTransformOrigin[index];
> +    if (originValue.GetUnit() == eStyleUnit_Calc) {
> +      const nsStyleCoord::Calc *calc = originValue.GetCalcValue();
> +      transformOrigin[index] =
> +        NSAppUnitsToFloatPixels((refBox.*dimensionGetter[index])(), appUnitsPerDevPixel) *
> +          calc->mPercent +
> +        NSAppUnitsToFloatPixels(calc->mLength, appUnitsPerDevPixel);
> +    } else if (originValue.GetUnit() == eStyleUnit_Percent) {
> +      transformOrigin[index] =
> +        NSAppUnitsToFloatPixels((refBox.*dimensionGetter[index])(), appUnitsPerDevPixel) *
> +        originValue.GetPercentValue();
> +    } else {
> +      MOZ_ASSERT(originValue.GetUnit() == eStyleUnit_Coord,
> +                 "unexpected unit");
> +      transformOrigin[index] =
> +        NSAppUnitsToFloatPixels(originValue.GetCoordValue(),
> +                                appUnitsPerDevPixel);
> +    }
> +  }

Please use a function to share this code with nsDisplayTransform::ComputePerspectiveMatrix

::: layout/generic/nsFrame.cpp:10534
(Diff revision 9)
> +  gfx::Matrix result2d;
> +  if (!matrix.CanDraw2D(&result2d)) {
> +    return gfx::Matrix();
> +  }

Seems less than ideal to just drop it on the floor if it has 3-D bits in it.  I guess it's chrome-only, though.  But maybe at least add a FIXME comment saying that it would have been better to reject 3-D components at parse time?  And perhaps also an NS_WARNING or a console message?

::: layout/generic/nsIFrame.h:3886
(Diff revision 9)
> +   * Computes a 2D matrix from the -moz-window-transform and
> +   * -moz-window-transform-origin properties on aFrame.
> +   * Values that don't result in a 2D matrix will be ignored and an identity
> +   * matrix will be returned instead.
> +   */
> +  Matrix GetWidgetTransform();

I'd call this ComputeWidgetTransform rather than GetWidgetTransform.  Calling it Get makes it sound like (a) it's fast and (b) it's getting something from the widget.
Attachment #8874172 - Flags: review?(dbaron) → review+
Thanks for the reviews!
Comment on attachment 8874170 [details]
Bug 1370034 - Add CSS properties -moz-window-transform and -moz-window-transform-origin, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145592/#review154708

> I'm presuming you generated these changes with "./mach devtools-css-db".  If not, you should have done it that way.  I'm not reviewing them for substance.  (Same for previous patch.)

Yes I did.

> Do you intend to accept 3-D transform functions here?  If so, what are you doing with them?

I'm accepting them and, very late in the pipeline, discard the whole transform (treat it as identity) if it can't render in 2D.

> You should do the same testing drill for these changes -- run mochitests with them not marked as internal.

Done (in the same try push).

> This method needs restructuring.  I should have caught this in the previous patch.  You need to do the right thing when multiple properties change, which this doesn't do.  You need to be able to send UpdateWidgetProperties hints along with others.
> 
> (Or is UpdateWidgetProperties somehow implied by SchedulePaint?)

No, UpdateWidgetProperties is not implied by SchedulePaint. What I did here is wrong, will fix.
Comment on attachment 8874172 [details]
Bug 1370034 - Call SetWindowTransform with the right values from -moz-window-transform(-origin).

https://reviewboard.mozilla.org/r/145596/#review154716

> Please use a function to share this code with nsDisplayTransform::ComputePerspectiveMatrix

I think the reason I didn't share it was because I'm treating the z component of the transform-origin differently. But that difference may have been misguided. I'll make another attempt to share it.

> Seems less than ideal to just drop it on the floor if it has 3-D bits in it.  I guess it's chrome-only, though.  But maybe at least add a FIXME comment saying that it would have been better to reject 3-D components at parse time?  And perhaps also an NS_WARNING or a console message?

I can add an NS_WARNING (which doesn't need to be localized) and the FIXME. I don't think it's worth spending time fixing it, though; the fact that these properties are chrome-only and can't even be used by WebExtensions makes these imperfections really insignificant.

> I'd call this ComputeWidgetTransform rather than GetWidgetTransform.  Calling it Get makes it sound like (a) it's fast and (b) it's getting something from the widget.

Good call.
Comment on attachment 8874167 [details]
Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145586/#review154730

::: layout/style/test/property_database.js:7908
(Diff revision 5)
> +  gCSSProperties["-moz-window-opacity"] = {
> +    // domProp: "MozWindowOpacity",
> +    inherited: false,
> +    type: CSS_TYPE_LONGHAND,
> +    initial_values: [ "1", "17", "397.376", "3e1", "3e+1", "3e0", "3e+0", "3e-0" ],
> +    other_values: [ "0", "0.4", "0.0000", "-3", "3e-1" ],
> +    invalid_values: [ "0px", "1px", "20%", "default", "auto" ]
> +  };
> +

There are a few test failures involving transitions on -moz-window-opacity. I'll look into them.
Comment on attachment 8874167 [details]
Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145586/#review154598

> I've started a try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd4b01a8541ee03a0a801a2de46e633fb76a97dd

Oh, the transition failures only happened because I hadn't adjusted test_transitions_per_property.html to expect these properties to be animatable. With that change, it passes.
Comment on attachment 8874172 [details]
Bug 1370034 - Call SetWindowTransform with the right values from -moz-window-transform(-origin).

https://reviewboard.mozilla.org/r/145596/#review154716

> I think the reason I didn't share it was because I'm treating the z component of the transform-origin differently. But that difference may have been misguided. I'll make another attempt to share it.

I created nsStyleTransformMatrix::Convert2DPosition and am now calling it both from nsDisplayTransform::ComputePerspectiveMatrix and from nsFrame::ComputeWidgetTransform.
Comment on attachment 8874167 [details]
Bug 1370034 - Add -moz-window-opacity property, style system parts (excluding stylo).

https://reviewboard.mozilla.org/r/145586/#review154598

> Oh, the transition failures only happened because I hadn't adjusted test_transitions_per_property.html to expect these properties to be animatable. With that change, it passes.

Final try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=b18c9c9a13a019db5cc25ef44acc371a6aebe184 . I had a few 3d coordinate values in the -moz-window-transform-origin test, but we only parse 2d values for the origin. Now that I've moved those 3d values to the invalid_values section, the tests finally pass completely.
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/c99450001fa1
Add -moz-window-opacity property, style system parts (excluding stylo). r=dbaron
https://hg.mozilla.org/integration/autoland/rev/28e733821d73
Add nsIWidget::SetWindowOpacity and implement it on Mac. r=spohl
https://hg.mozilla.org/integration/autoland/rev/46c5f91672a9
Implement UpdateWidgetProperties for top level windows and for popups, and call nsIWidget::SetWindowShadow. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/aa5be78cd9fc
Add CSS properties -moz-window-transform and -moz-window-transform-origin, style system parts (excluding stylo). r=dbaron
https://hg.mozilla.org/integration/autoland/rev/f84d7ac5105d
Add nsIWidget::SetWindowTransform and implement it on Mac. r=spohl
https://hg.mozilla.org/integration/autoland/rev/193433849675
Allow disabling window transforms using a pref. r=spohl
https://hg.mozilla.org/integration/autoland/rev/37c332642a18
Call SetWindowTransform with the right values from -moz-window-transform(-origin). r=dbaron
Depends on: 1374177
Depends on: 1374178
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/5fced871414b
Add -moz-window-opacity property, style system parts (excluding stylo). r=dbaron
https://hg.mozilla.org/integration/autoland/rev/09f895ddacdc
Add nsIWidget::SetWindowOpacity and implement it on Mac. r=spohl
https://hg.mozilla.org/integration/autoland/rev/2cf330925da7
Implement UpdateWidgetProperties for top level windows and for popups, and call nsIWidget::SetWindowShadow. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/1e5fe4feb659
Add CSS properties -moz-window-transform and -moz-window-transform-origin, style system parts (excluding stylo). r=dbaron
https://hg.mozilla.org/integration/autoland/rev/d40fdf7c04eb
Add nsIWidget::SetWindowTransform and implement it on Mac. r=spohl
https://hg.mozilla.org/integration/autoland/rev/3c2d465f021e
Allow disabling window transforms using a pref. r=spohl
https://hg.mozilla.org/integration/autoland/rev/d19edbac8bc9
Call SetWindowTransform with the right values from -moz-window-transform(-origin). r=dbaron
Blocks: 1375232
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: