I don't think you even had to restructure it this much, could have just move the top portion (based on whether it's dynamically encoded or not) below the "is value type -> let := (memberSrcPtr)", i.e.

<?fromCalldata>
	<?isValueType>
		let <memberValues> := <read>(memberSrcPtr)
	<!isValueType>
		let <memberValues> :=
			<?dynamicallyEncodedMember>
				<accessCalldataTail>(value, memberSrcPtr)
			<!dynamicallyEncodedMember>
				memberSrcPtr
			</dynamicallyEncodedMember>
	</isValueType>
</fromCalldata>

Would have made for an easier diff I'd say, but your implementation is semantically equivalent, so no biggie.

Gonna have to move this up into 0.8.37.

I would add a few more tests:

1. struct with external function ptr + uint64 (packing)
2. struct with external function ptr + uint256[] array (dynamic)
3. array of structs with external function ptr, i.e.

contract C {
  S[] storageStruct;
  ....
}