[Model 2] Unoptimized preliminary support for mipmaps and trilinear filtering #14318
Conversation
1. Code standard 2. Anti Alpha higlighted edges ala Supermodel 3. Cleaner bilinear edge cases ala Supermodel
…ap lod index per pixel [model2.h and model2_v.cpp] Added all mipmap information to m2_poly_extra_data structure [model2rd.ipp] Added preliminary unoptimized support for mipmaps and trilinear filtering
| // 6 MIPS LEVELS | ||
| // EACH MIP LEVEL HAS HALF WIDTH AND HALF HEIGHT | ||
| // LOCATED RECURSIVELY IN THE BOTTOM RIGHT CORNER OF 2048x1024 | ||
| // EACH LEVEL HAS FLIPPED RAM BANKS |
There was a problem hiding this comment.
Can we please normalize this comment?
There was a problem hiding this comment.
Yes absolutely. Just to be clear, what do you mean by normalizing? Just removing the uppercase or enclosing by /* */ or something else? I see multiple conventions in place line 2526 of model2_v for example is lowercase surrounded by /**/, line 2645 is multi-line //.
Would below work?
// 6 mips levels
// each mip level has half width and half height
// located recursively in the bottom right corner of 2048x1024
// each level has flipped ram banks
There was a problem hiding this comment.
Normalize: don't use screaming upper case, particularly for an informative comment.
There was a problem hiding this comment.
Thanks for the feedback, will fix it shortly.
There was a problem hiding this comment.
Done. #14326
Let me know if you need anything else.
There was a problem hiding this comment.
Actually in English it is standard practice to capitalize the first letter of a sentence and put a full stop at the end of a sentence. Also MIPS should be all capital since it's an abbreviation and RAM should also be all capitals. So technically it's still not correct ;-)
/*
6 MIPS levels.
Each MIP level has half width and half height located recursively in the bottom right corner of 2048x1024.
Each level has flipped RAM banks.
*/
There was a problem hiding this comment.
Thanks for the advice, I will keep it in mind for my next update. At the same time, if you search for mip/Mip/ram/Ram/rom/Rom in the entire code base there seem to be plenty of code that could benefit from the same advice. Is this recommendation codified somewhere in a kind of coding standard? If yes I need to read it. If not maybe it would be worth considering.
There was a problem hiding this comment.
Haha yes. Again not sure if it is proper English (and which one British/American/Australian?) but mips as the plural shorthand for mipmaps is used widely in the game industry. Even found it in Unity's docs (https://docs.unity3d.com/2020.3/Documentation/Manual/texture-mipmaps-introduction.html) and nvidia for example (https://forums.developer.nvidia.com/t/tutorial-creating-and-modifying-custom-mipmaps/202234)
You guys let me know what you want and I will happily change it.
There was a problem hiding this comment.
Ok then using mip/mips is incorrect and it should be...
6 mipmap levels
Each mipmap level has...
etc
|
Comments are not code nor is there any standard way to write them. However MAME is an English project and as such you should adhere to English writing standards when writing comments. Or something like that. Because there are people working on MAME from all over the planet where some of them don't use English as their first language there can be a bit of randomness to comments in the source. Occasionally when there is enough cosmic radiation at the highest levels there can be brief spurts of corrections across various source files but often there are many remaining that are not corrected. With such a large project like MAME it's a full-time job just to maintain the comments and that's not really a priority. |
4 participants
[poly.h] Added dpdy information to extents to be able to compute mipmap lod index per pixel
[model2.h and model2_v.cpp] Added all mipmap information to m2_poly_extra_data structure
[model2rd.ipp] Added preliminary unoptimized support for mipmaps and trilinear filtering