Nit: In the documentation, can you mention this is the same as column-major storage. I know the arguments are somewhat descriptive but a quick note here would make it easier to read.

This is actually row-major, though.

But I'll add some reminder comments.

Doh, you're right. The updated comment is clear enough.

I am probably misremembering the action items from our conversation the other day. But, shouldn't we just avoid using SkMatrix altogether? It brings nothing to the table except minor storage gains (relative to everything else in the display list) but causes subtle issues in corner cases. Let's just use a single consistent POD type for matrices. It ought to simplify a whole heck of a lot of code and cause less confusion in the process.

The documentation is there on the accessor methods in the base class (SkMatrixSource). For a single transform operation, SkMatrix works fine. If you work out what happens when you transform (x, y, 0, 1) and only need the x,y from the result, SkMatrix has all of the entries you need.

The important thing is that you accumulate the 4x4 for all transform mutations, but then you can use the SkMatrix for transforming points and rects. Also, on a more practical note, that SkM44 doesn't have any mapping methods.

I'll update the block comment in the Dispatcher block comment with the specifics of the math. And maybe sprinkle in some reminder comments.

With regards to storage, 99.9% of the matrices we deal with are 2x3. Also, for the code reading and analyzing a DisplayList it helps to know up front if the matrix operations are all 2D affine. Once a transform4x4() method is dispatched it begs the question of whether or not 3D operations are happening without examining the values. For now, the characterization of the matrix into 1 of 3 categories happens in the Builder so the DisplayList streams can trust the categorization without any redundant characterization code.

There is an argument to be made for eliminating the transform3x3. If someone introduces a perspective transform into the mix then a 3x3 may be enough for leaf transform operations, but it is not enough for further composition. Just as the engine, over time, came to trust an SkMatrix for its matrix operations even though it cannot compose multiple non-2D-affine transforms, having that dispatch method may come to have the same effect on future uses of the DisplayList - "hey they take a 3x3 and it has perspective entries, so it must be something that I should use!". Better to set the precedent now to only deal with 2x3 (extremely common 2D affine) or 4x4 so that if you even think of embracing perspective then you do so with the more responsible 4x4.

Sounds good on the arguments. About SkM44 not offering the mapping methods, perhaps we should also file issues for Skia to implement them?

I was going to add that request, but also to think of suggestions for how the API docs and positioning of the APIs can be enhanced to discourage the kind of misuse we've had. There are still a number of places in SkCanvas where you can helpfully hand in an SkMatrix (i.e. 3x3) that will be composed with the canvas matrix, but does it really have enough precision? I see they've deprecated getTotalMatrix() which returns a subset SkMatrix and the new methods require you to deliberately request a 3x3. I would go a step further and not have get...AsM33() and force the caller to get...().asM33() so it is even more deliberate that they are subsetting the answer (though it would require more copying of data). If SkM44 provided mapping methods, it is unlikely that method would be needed.

Another option is to (would have been to) have a wrapper class SkTransform which provides the utility methods and does not indicate whether it is 2x3, or 4x4. Underneath it will maintain whatever precision is needed for its history of operations. Then you can get the efficiency of 2x3 implementations if your (very common) app is all 2D affine and only pay the price of 4x4 when it is needed.

Also, the commit that eliminates the 3x3's has just been pushed.

Doh, you're right. The updated comment is clear enough.

Sounds good on the arguments. About SkM44 not offering the mapping methods, perhaps we should also file issues for Skia to implement them?