All the ConvertTo() methods in WritingModes.h swap coordinates whenever the source and target writing modes are different. For LogicalSize this can be improved: when converting between two horizontal or two vertical modes the result will be the same so we can skip the conversion.

The simple patch that I envisaged for this doesn't work because the writing modes are still different so we assert all over the place (https://tbpl.mozilla.org/?tree=Try&rev=0773e5d2181b). We could in theory take this a step further by relaxing the CHECK_WRITING_MODE assertions on LogicalSize to check only for the same axes and not the same directions, but I don't think that's a great idea because the assertions are valuable for catching logical errors even when the results of calculations aren't affected. Alternatively we could do what this patch does, but maybe it's not a significant win any more and we can just WONTFIX this. WDYT?

Comment on attachment 8529707 [details] [diff] [review] Patch Review of attachment 8529707 [details] [diff] [review]: ----------------------------------------------------------------- My gut feeling is that this version should be better that the current code, as it does the conversion (or non-conversion) more directly, so we may as well do it. (I suppose micro-benchmarking and/or comparing the compiled code that we get in an optimized build would be the way to really know.) ::: layout/generic/WritingModes.h @@ +769,5 @@ > CHECK_WRITING_MODE(aFromMode); > + return aToMode == aFromMode > + ? *this : aToMode.IsOrthogonalTo(aFromMode) > + ? LogicalSize(aToMode, BSize(), ISize()) > + : LogicalSize(aToMode, ISize(), BSize()); I suppose we could even do something like return aToMode == aFromMode ? *this : aToMode.IsOrthogonalTo(aFromMode) ? LogicalSize(aToMode, BSize(), ISize()) : #ifdef DEBUG // ensure correct writing-mode in the result LogicalSize(aToMode, ISize(), BSize()) #else // in non-DEBUG builds, LogicalSize does not store the writing mode, // so we can simply return the existing value *this #endif ; but maybe that's too tricky to be worth it for the marginal gain?

Yes, I like that idea, but I decided to implement it slightly differently, thus:

Comment on attachment 8530308 [details] [diff] [review] Patch v.2 Review of attachment 8530308 [details] [diff] [review]: ----------------------------------------------------------------- Yes, that's more readable than what I wrote!