Just to clarify, I'm not talking about changing how the matrix is stored internally. Column-major makes sense to me. I just prefer to change the order of the template parameters.

For example, float3x4 is a matrix with 3 rows and 4 columns to me. I think that makes sense because I think of float3 as a column vector, and I have 4 of those.

From Eigen-only code perspective I agree with you. But in realities of Blender I am not convinced this is great . It opens the opportunities to make a lot of mistakes during conversion stages, or when one works with both C and C++ code: you'd constantly need to think whether it is [col][row] or (row, col) or (col, row) or something else. This is intrusive enough decision, and it at least needs to be supported by the Core module.

Disallowing subscript operator goes agains the part of the design which talks about aligning GLSL syntax with C++. The amount of code duplication between C/C++ and GLSL is to be minimised.

mat.col(x, y) is unclear to me what it is.

mat.col(x, y) is unclear to me what it is.

Oh oops, that was a typo.

Yeah, I can see that changing [col][row] or so to something else can cause many bugs, hence my suggestion to not allow it at all. But yes, I forgot about the GLSL compatibility. Makes me wonder whether we should not use this kind of access in glsl either.

That isn't required since none of the copy / move constructor/assignment are defined. Thus these are automatically defined.

enable_if will not give human understandable clues about why my_matrix.forward() is undefined. I would much rather see a clue "Hey, expecting number of rows to be at least 3" than "hey, undefined reference to forward, and here is a kilometer of unreadable substitution attempts".

In general, I think majority of BLI_ENABLE_IF in BLI is just plain wrong. I would really suggest to stay away from it, and start thinking of a good flexible API.

This is in order to provide template specialization for common types. I couldn't find a nice way for friend functions to work with this.

I did it out of simplicity and avoiding to template this even more. I'll leave that as a TODO.

Good catch!

This patch uses the same hash as the old float4x4 library for compatibility. If we have to change it, it isn't in this patch.

I find that's a little less obvious.

Your example does not compile. (MacOS + Clang 13.1.6)

You should get something like

/Users/clement/Blender/blender/source/blender/blenlib/tests/BLI_math_matrix_types_test.cc:298:32: error: no viable overloaded '+='
  mat_const.view<2, 2, 1, 1>() += 5;
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~
[...]
/Users/clement/Blender/blender/source/blender/blenlib/BLI_math_matrix_types.hh:566:12: note: candidate function not viable: 'this' argument has type 'const MatView<float, 2, 2, 4, 4, 1, 1>', but method is not marked const
  MatView &operator+=(T b)

This is because the view() method return either a const MatView or a MatView depending on the object const type.

So I am a bit hesitant to make the code way less understandable for something that should work.

There is an important difference between:

mat_const.view<2, 2, 1, 1>() += 5;

and

auto view = x.view<4, 4>();
view += 5;

The example you provided indeed does not work, but my example does and it has the const correctness problem.
The problem is that a const MatView can just be copied (it's just a pointer after all) and then it is not const anymore. This is what happens in my example. x.view() returns a const MatView, but view is just a MatView.

Does not look like this has been addressed.

This is already the case here. You were just confused by the long list of arguments.