1766078 - Add a ViewAs overload for casting an untyped ScaleFactors2D to a typed one

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task, P3)

Description

[Botond Ballo [:botond]]

We have a series of ViewAs overloads for casting untyped quantities to typed quantities, but we don't have one for ScaleFactors2D.

Such a function would be usefull to do a bit of further cleanup after bug 1733313.

Comment 1

[Razvan Cojocaru]

If this is specifically about bug 1733313, the problem there is not that there's no ViewAs() template specialization for ScaleFactors2D, but rather that there's no operator*= at all written for it. In fact, adding a properly written one should make the need for ViewAs() go away, with the added benefit (IMHO, at least) of cleaner code. Here's what I mean:

diff --git a/gfx/2d/ScaleFactors2D.h b/gfx/2d/ScaleFactors2D.h
--- a/gfx/2d/ScaleFactors2D.h
+++ b/gfx/2d/ScaleFactors2D.h
@@ -113,6 +113,14 @@ struct ScaleFactors2D {
     return *this * ScaleFactors2D<other, src>(aOther);
   }
 
+  template <class othersrc, class otherdst>
+  ScaleFactors2D<src, dst>& operator*=(
+      const ScaleFactors2D<othersrc, otherdst>& aOther) const {
+    xScale *= aOther.xScale;
+    yScale *= aOther.yScale;
+    return *this;
+  }
+
   template <class other>
   ScaleFactors2D<src, other> operator/(
       const ScaleFactor<other, dst>& aOther) const {
diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1086,10 +1086,7 @@ void nsImageFrame::MaybeDecodeForPredict
 
   // If we are in a remote browser, then apply scaling from ancestor browsers
   if (BrowserChild* browserChild = BrowserChild::GetFrom(presShell)) {
-    resolutionToScreen.xScale *=
-        browserChild->GetEffectsInfo().mRasterScale.xScale;
-    resolutionToScreen.yScale *=
-        browserChild->GetEffectsInfo().mRasterScale.yScale;
+    resolutionToScreen *= browserChild->GetEffectsInfo().mRasterScale;
   }
 
   // ...and this frame's content box...

Comment 2

[Razvan Cojocaru]

Oops, I've copied and pasted operator*='s signature from operator* and forgot to remove the const, but it all compiles and appears to work with the const removed, and my observation still stands.

Comment 3

[Botond Ballo [:botond]]

You're right that adding an operator*= like this would compile. However, that would weaken our strongly typed unit system, because we could now multiply e.g. a LayerToScreenScale2D with a completely unrelated scale like CSSToLayoutDeviceScale2D.

A ScaleFactors2D<A, B> converts from coordinate system A to coordinate system B. So, it makes sense to multiply ScaleFactors2D<A, B> and ScaleFactors2D<B, C>, resulting in a ScaleFactors2D<A, C>, that effectively chains the two conversions. However, it does not make sense to multiply ScaleFactors2D<A, B> and an unrelated ScaleFactors2D<C, D>. Our current operator* reflects that (only allowing the first kind of multiplication).

I would like to keep this constraint in place, and use the existing operator* to write the resolutionToScreen * mRasterScale multiplication. This is where the ViewAs() function comes in: since mRasterScale is an "untyped" scale (the source and destination units are UnkownUnits), we can "cast" it to a scale with the proper units that make the multiplication compile (in this case that would be ScreenToScreenScale2D. Our convention is to perform such casts explicitly by calling the ViewAs() function, but it needs an appropriate overload for casting ScaleFactors2D.

(A separate point is that our existing operator* does not have an operator*= version. We could add one (if so, we should add it for all the operators for consistency), or we could just write resolutionToScreen = resolutionToScreen * mRasterScale at the call site).

Comment 4

[Razvan Cojocaru]

Attachment: Bug 1766078 - Add a ViewAs overload for casting an untyped ScaleFactors2D to a typed one. r?botond

Please note that ScaleFactors2D is missing operator*=() for now,
hence the "resolutionToScreen = resolutionToScreen * X" formula.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5f361b1b1d4
Add a ViewAs overload for casting an untyped ScaleFactors2D to a typed one. r=botond

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a5f361b1b1d4