fix bug in calculus in texture_font.cpp #2

Thanks for the formula simplification, I did not catch that

Only now i have realized that I mixed up rows and columns here. mRows should calculate the amount of horizontal character cells and mColumns the amount of vertical character cells :P

rowIndex and columnIndex names also seem to be mixed up.

You can fix both of those names here and in GetCharacterRegion if you want. If not, I'll change it myself in the future. It won't change the logic, it just will be clearer.

We do std::floor here because if the division ends up being closer to the ceiling than the floor C++ will implicitly round up when converting to an unsigned integer. This behavior may return the wrong amount of rows and causing bugs if the font text has more pixels than the minimum necessary.

Take the current assets/monogram-bitmap.png texture we test on for example: it is currently 96 pixels wide, the character width is 5 and the horizontal spacing between characters is 1, each row has 16 columns of characters. As it stands, it correctly calculates 96 / (5 + 1) = 16 and still implicitly correctly rounds down (without std::floor) to 16 until the user puts an image with a width of 99 (99 / (5 + 1) = 16.5 which will correctly round to 16).

But what happens if the user accidentally uses a texture with 4 extra pixels? texture->width will be 100 and the result will be 100 / (5 + 1) = 16.666 which will implicitly round up to 17. But that would be a bug, because that texture can't possibly contain 17 characters horizontally (with mCharWidth = 5 and horizontalSpacing = 1). To prevent that we always explicitly round down with std::floor.

Please keep the std::floor call around both of these lines.

Thanks for catching that possible divided by zero bug! I wouldn't have gotten that.

Although I would ask you to include explicit braces here and put mColumns = 1 in its own line. readability-braces-around-statements is a check enabled on .clang-tidy, and I personally prefer for it to be explicit.

Also my fault. I need to write some contributing guidelines and require clang-tidy checks.

Same possible rounding bug as mentioned before. std::floor should be kept.

I understood why you changed it to mColumns here, since it's calculating based on the columns. But that would end up being straight up incorrect code because I mixed up mRows and mColumns names before :P. If you decide to switch mRows and mColumns names everywhere else, you may keep mColumns here. Otherwise, keep the original mRows, as it would be a bug to not do so.

Also, you misspelled columnIndex as columIndex, but that is not as significant :)

Same std::floor and mColumns/mRows name switch comments here. Otherwise, thanks for the simplification, I would not have gotten that :)

You seemed to have misspelled here, mCharWidth should be mCharHeight :)

These lines do not respect the 80 columns default limit imposed by clang-format, no big deal there though. But clang-tidy rightfully points out narrowing conversions in the static_casts here. The original code does not have different types on both sides of the division, so no static_cast is needed, and as clang-tidy points out, it might make a narrowing conversion to happen. So remove those.

The code base has both .clang-tidy and .clang-format files in it. Please use them and always run clang-tidy and clang-format before commits and apply the suggested fixes. It keeps the style and opinions about conventions and formatting consistent across the code.

If it's a truly nonsensical thing clang-tidy is complaining about, you can ignore it inline or in .clang-tidy/.clang-format and argue why it would be better to ignore it. I already do ignore a ton of stuff on there, but I quite like the majority of the defaults. But nothing that clang-tidy is saying here is nonsensical.

Most importantly, you did not address the rows/columns naming confusion (that was present prior to your changes) that causes the calculations to simply not work after your changes. I already addressed them and where exactly you should change them for the code to work. But as it stands this code does still not work. The texture_font_test still incorrectly renders the "Hello World" text. Simply addressing the rows/columns naming confusion makes it work again.

It seemed like you again did not test your changes before commiting code. Please always run the tests relevant to the changes to make sure your code does work as intended before commiting.

Same three points discussed before also apply here. clang-format, clang-tidy and rows/columns naming. Run the tests before commiting.