XbSymbolDatabase icon indicating copy to clipboard operation
XbSymbolDatabase copied to clipboard

Fix LTCG Versioning Method

Open RadWolfie opened this issue 5 years ago • 7 comments

Due to D3D8LTCG's OOVPA signatures using two separate versioning which is understandable. However, the signatures with version "1024" appear are not always in 3911's database. Which I had noticed and so are the other versions too.

I suggest a better versioning method to keep the LTCG's database sane and easier to find.

https://github.com/Cxbx-Reloaded/XbSymbolDatabase/blob/76146fa05dca5ce0b472e586c6eb1e518fc32743/OOVPADatabase/D3D8LTCG_OOVPA.inl#L39-L41

My 2 proposals are:

  • Keep the versioning method, except change 1XXX and 2XXX to use first 3 numbers. Such as 1391 (3911), 1392 (3925), 1403 (4034), and so on.

OR

  • Add "LT(CG)" at the end of the function's symbol for 2XXX only. I have an idea to able separate D3D8 symbols and D3D8LTCG by add D3D8LTCG flag to existing D3D8 database.
    • Doing this way will allow full version (3911) support for both 1XXX (without "LT(CG)") and 2XXX (with "LT(CG)"), I think. I'll need to try it out later.

TASKS:

  • Create a branch from this repository to contain __LTCG prefix and use actual versioning from existing version files.
  • Create a draft pull request to Cxbx-Reloaded, or use XbSymbolDatabaseTool from here, for any regression may be found.
    • This will be the point we will need to rely on testers with LTCG titles response back with before and after cache/log file.

RadWolfie avatar Jun 04 '19 17:06 RadWolfie

I've left some comments via Discord. But they didn't find their way into this issue:

Instead of hacks like this 1xxx or 2xxx, just fix it properly, please. Even kernel version 6000 would only be ~0x1770; so bits 0x2000, 0x4000, 0x8000 remain untouched.

It could become OPTIMIZED | 3911 (instead of 2xxx ~ 2911) or RECOGNIZED | 3911 (instead of 1xxx ~ 1911). So it would add flags / state to the unused bits:

  • 0x0000 [00]: non-LTCG
  • 0x4000 [01]: OPTIMIZED
  • 0x8000 [10]: RECOGNIZED
  • 0xC000 [11]: remains unused

You could also always add 0x2000 as another flag (or fall-back to it, if 0x8000 is already used - I didn't check); 0x2000 either as boolean, or as extension for state number, giving you 8 options when combined with 0x4000 and 0x8000.

It's basically the same as 1xxx / 2xxx (encoding multiple things in one field), except my suggestion is more readable and less error prone (avoids accidental match of last 3 digits).

Obviously, you could also turn these flags into macros: OPTIMIZED(3911) if that's considered more readable.

If the codebase already uses bitmasks in the upper bits and has no room for more flags, the entire encoding could be changed to something like VERSION_3911 (which would refer to an index, instead of a 4 digit number for the first version).

JayFoxRox avatar Jun 07 '19 10:06 JayFoxRox

Adding the flags will not help. The symbol cache doesn't have any "flag" support and shouldn't provide support for it anyway. Also, generated symbol strings are output differently for LTCG.

My second proposal is closely identical to your suggestion, @JayFoxRox. Except, it will break Cxbx-Reloaded. Meaning, Cxbx-Reloaded will need an update to match the symbol renames.

RadWolfie avatar Jun 07 '19 14:06 RadWolfie

We've discussed this on Discord again.

My proposal wouldn't work, because the Version number is used as a string as part of the macro. So math can't be done:

https://github.com/Cxbx-Reloaded/XbSymbolDatabase/blob/c89ee864269711491e710ada252b0f301258443d/OOVPA.h#L160-L161

Additionally, the version field might only be 13 bit (although the comment is apparently outdated):

https://github.com/Cxbx-Reloaded/XbSymbolDatabase/blob/c89ee864269711491e710ada252b0f301258443d/OOVPA.h#L152


Since then, @RadWolfie has also clarified that the original post contains 2 different proposals. The second proposal doesn't cause these issues. My understanding is that it moves these markers into the symbol name (which is also a bit dirty, and might have some drawbacks / but also some benefits). It would essentially add the functions as a separate function handler.

JayFoxRox avatar Jun 07 '19 16:06 JayFoxRox

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 1024, 1036, 1048, 1060),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_12, 2024),

Convert above to below as like proposal 2

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 3911, 4034, 4627, 5849),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_LTCG_12, 3911),

or something @JayFoxRox suggest to use the LTCG at the end of the symbol.

REGISTER_OOVPAS(D3DDevice_DrawVerticesUP, 3911, 4034, 4627, 5849),
REGISTER_OOVPAS(D3DDevice_DrawVerticesUP_12_LTCG, 3911),

Plus he also advise to use double underlines, __ for any numbers/"LTCG" after the mangle symbol name.

RadWolfie avatar Jun 07 '19 16:06 RadWolfie

Just want to share my discovery when worked on D3D's render state variables pull request #177. I notice there are functions that are exactly the same except shifted by one to five offsets, sometimes more than five. Which is majority affecting LTCG titles. Due to current and future versioning' limitation, I think we should re-haul OOVPA design to something else*. Doing so will cut down redundant signatures that are only shifted one to five offsets into one simple signature. Then future versioning design could work better.

* Something else could be like tuple design which could transform into custom instruction internally. How would it work is a mystery for me. However, this type of work will require backward compatibility removal, aka OOVPA(_NO)_XREF, first.

RadWolfie avatar Jan 24 '23 03:01 RadWolfie

Similar to RadWolfie's first proposals, but adding a minor number. It also avoids conflicts between D3D and D3D8LTCG 1xxx. Such as

D3DDevice_DrawIndexedVerticesUP, 3911
D3DDevice_DrawIndexedVerticesUP, 1024
D3DDevice_SelectVertexShader_0, 2072
D3DDevice_SelectVertexShader_0, 2084
D3DDevice_DrawIndexedVerticesUP, 3911
D3DDevice_DrawIndexedVerticesUP, 3911.1
D3DDevice_SelectVertexShader_0, 5849.1
D3DDevice_SelectVertexShader_0, 5849.2

jarupxx avatar Jan 28 '23 12:01 jarupxx

@jarupxx, I am not seeing how minor number will work with current implementation nor with group of symbols in the database list.

RadWolfie avatar Feb 09 '23 09:02 RadWolfie