Should we update those two just in case?
https://github.com/Jjagg/MonoGame/blob/cb24746589835f0d4e2b7c041b6b0c8659979737/Tools/2MGFX/EffectObject.writer.cs#L159-L160

That's not necessary, row and column are never more than 4 (we could even encode them in a single byte actually).

Oh, I see. you are right.

Read7BitEncodedInt and Write7BitEncodedInt are protected, so I had to create subclasses to use them in a proper way. Not sure why they're protected, I couldn't find a good explanation anywhere. Anyway, just subclassing seemed like a good solution to me. This is good to go as far as I'm concerned.

Interesting, I didn't know that. The 7BitEncodedInt is used in the ContentWriter for some parts of XNB.
In the XNB Format documentation:

This format matches the .NET BinaryReader.Read7BitEncodedInt method. It is a variable size encoding of a 32 bit integer value. C implementation:

int Read7BitEncodedInt()
{
    int result = 0;
    int bitsRead = 0;
    int value;

    do
    {
        value = ReadByte();
        result |= (value & 0x7f) << bitsRead;
        bitsRead += 7;
    }
    while (value & 0x80);

    return result;
}

Oh, cool! I could just implement it like that if that's preferable.

Well, it doesn't give the implementation of Write7BitEncodedInt(), something that you need to figure out.
Anyway will work IMO.

Yeah, I think this is fine as is. MG also uses the 7 bit encoded int methods in the ContentReader and ContentWriter classes, but they extend BinaryReader and BinaryWriter, so they also use the .NET implementation.

@tomspilman @cra0zy This is still good to go :) Heads up, it updates the MGFX version. That's important for release notes.

Will the mgfxo be up-to-date, no rebase needed? (I haven't been following much the repo history lately, just a safe check. :) )

@mrhelmut Yes, the MGFX version has not been updated by another PR (it's still 8 in develop) which means the binary format has not changed since this branch was created. I recompiled all effects in my last commit, so the .xnb files are up to date :)

https://github.com/Jjagg/MonoGame/blob/0c416a94b9e4b256be0c8912ad6f49218a67be3f/MonoGame.Framework/Graphics/Effect/Effect.cs#L244

var sizeInBytes = (int)reader.ReadUInt16();

A checked out version of this will still crash when using arrays with size > 1541

ShaderCode (crash)
#define SIZE 1542
float indexList[SIZE ];

ShaderCode (will work)
#define SIZE 1541
float indexList[SIZE ];

It will crash at Ln49 of ConstantBuffers.cs with an System.OverflowException: 'Arithmetic operation resulted in an overflow.' because the sizeInBytes is negative.

The real problem seems to result from ln244 of Effect.cs where var sizeInBytes = (int)reader.ReadInt16 (); results in a negative number.

Looks like Jjagg's commits are effective refactoring. I'd support pushing to base.

Yes, I don't remember looking into the 1541 limit for arrays mentioned above, but this still at least improves the limit. This PR is old, but still good to go.

@harry-cpp This one is good to go!
Need to put in the release notes for 3.8 that people should recompile their effects after upgrading because the format changed.