bo3b / 3Dmigoto

Chiri's DX11 wrapper to enable fixing broken stereoscopic effects.
Other
770 stars 122 forks source link

INI global variable float parsing doesn't fail properly #111

Closed FinalDoom closed 5 years ago

FinalDoom commented 5 years ago

https://github.com/bo3b/3Dmigoto/blob/745231bad149b369f2ed114e7258cafb1e615093/DirectX11/IniHandler.cpp#L2083

I'm not sure this check has the effect it was intended for. It catches things like trying to parse strings, but if you pass in an 8 digit number, it passes, even though the float returned has 7 digits of precision -- two 8+ digit numbers that start with the same digits will parse as the same number, even though they're not. Eg. the returns of 99999991 and 99999995 are the same.. This is hard to debug if you don't know that variables are limited to 7 digits.

DarkStarSword commented 5 years ago

The check is intended to catch typos and incorrect use of comments, not precision issues.

It is incorrect to measure floating point precision in terms of decimal digits - floating point values have 24 bits of precision in binary. Further, applying any sort of check to catch these is bound to hit edge cases that you won't have considered - case in point, 0.2 is trivial to represent in decimal, but cannot be represented precisely in binary as you end up with a repeating sequence of bits (0x3e4ccccccccc....) which will naturally lose accuracy after the 24th bit rounds up to d.

FinalDoom commented 5 years ago

I guess a use case and a reference where I got that incorrect terminology may have been helpful. I was trying to understand what sort of data global variables take, and as it turns out it's only floats. Not a big deal, as they have a pretty big range, or so I thought. I thought I had read this in the M$ documentation for the format function, but I guess it was from here https://www.geeksforgeeks.org/difference-float-double-c-cpp/ that you only get 6-7 decimal digits of precision. Anything beyond that is just discarded. So while I can go up to a number much larger than a normal max int, it's not useful for uniqueness.

Use case: I'm trying to outline a way for modders to assign their mods unique ids to prevent conflicts. Something that'd make sense is like IIIIYYMMDDSSS (III modder id they choose, obvious date, and SSS=sequence on that date) so you don't have to keep track of all the mods you've written, since there's a date in there.

However, since that's over 7 digits, you only get IIIIYYM -- which is absolutely not unique. As a compromise I've settled on IIyyySS where yyy is day of year, but that's not exactly friendly to most users, and 99 is a low (though not totally unrealistic) limit on number of modders. It's possible but pretty unrealisitc to have id conflicts with something like that. I understand float weirdness makes that tough to check for. It's just hard to debug if you don't realize that your int-looking number is being parsed as float, to essentially only 7 digits of precision.

A nicer solution would probably be just allowing ints, longs or strings, or even just doubles instead of floats. But I get there's probably a significant performance penalty therein.

Additional question, is there a document somewhere in the repo that outlines the INI language? E.g. while trying to figure out variable capabilities, I experimented with keywords (global, pre, post, tried local, etc.). I get how global works now, and that local is just a no prefix variable in a section. I don't really get how pre/post work, if they even still do, or if there are other possible keyword configurations hidden somewhere.

DarkStarSword commented 5 years ago

If you want to represent integers, the maximum number you can safely use is pow(2,24), i.e. 16777216 (and the lowest number you can use is -16777216)

Floating point can represent significantly higher values than that, but you start getting gaps at the integer level, i.e. the next highest integer that it can represent is 16777218, as 16777217 (and all odd numbers thereafter) would have required 25 bits of precision. Notably you can safely represent any exact power of 2 up to pow(2,127) (and down to -pow(2,126)) since these all only use a single bit of precision.

We use floats because they are the main data type usable on the GPU*, and if any of these are assigned to IniParams, etc they have to be floats. It is possible that we might add additional data types in the future, but these would (at least initially) be intended for special cases, such as directly comparing against texture hash values (and this case would be specified as hexadecimal with the 0x prefix), but even that would give you a way to use 32bits of precision so I will keep your use case in mind (depending on how these end up being implemented you may have to use the === and !== operators rather than == and != since floating point equality tests have some special cases for NAN and INF values).

I do have plans to allow StructuredBuffers layouts to be specified in the ini file in the future and allow easier assignment to the fields in these, which would benefit from being able to use mixed data types as well - however it is not a given that the initial implementation of this would allow these data types to be specified directly or whether they would be specified as floats and then cast. This support is a while off as there are several other prerequisites we need before we can realistically consider adding it.

* GPUs can use other data types, but as you suspected do incur significant performance penalties, as well as other complications such as having to store double precision values split across two 32bit register components and having to bring in the special HLSL intrinsic asdouble() / asuint() functions to pack and unpack them from data storage. 3DMigoto's assembler only gained support for these data types recently and the decompiler still lacks support for them. To date we have only ever encountered a single game (Resident Evil 2 remake) that uses double precision, and only in a single shader.

DarkStarSword commented 5 years ago

A bit of a crazy idea, but you could exploit the floating point properties for your use case by making separate use of the exponent and mantissa. Playing around a little, a formula like this could work:

mod_id = 2**(modder_id - 149) * (2**23 + YYMMDDS)

Where modder_id is between 0 and 253 inclusive (or get rid of the -149 and make it between -149 and 104 inclusive, or just tell people to choose a value between 0 and 100 for simplicity). The addition of the 2**23 here forces the use of the low order bits in the mantissa, so YYMMDDS will always appear in the low order bits of the resulting float and avoids cases where the floating point representation of different date and sequence numbers would adjust the exponent in ways that could lead to collisions. (Another way of thinking about this is that we are explicitly specifying the (usually implicit) high order 1 bit of the mantissa to pin the precision where we want it).

The exponent won't quite match 1:1 to the modder_id because we need to avoid subnormals in the final value (i.e. the biased exponent must be >= 1) to ensure the mantissa won't change meaning that would mess up the other half of the formula (and likewise we need to avoid the biased exponent being 255 since that is NAN/INF territory). The -149 here comes from the exponent bias (-127), subtracting the highest power of 2 in the mantissa (-23) and adding 1 to avoid subnormals.

The negative bit is unused here, but could be used to double the amount of available modder_ids or sequence numbers if needed.

Note that I've only theory crafted this so far (and done some preliminary testing in Python), and would want to verify that there aren't any edge cases I haven't considered that could trip up the calculation in 3DMigoto.

Note that the initialiser on global variable declarations does not currently support expressions (I could probably relax this to allow statically evaluatable expressions such as the above, or implicitly defer non-statically evaluatable expressions until execution time, but for now...) so this would need to be done in two lines:

[Constants]
global $mod_id
$mod_id = 2**(MODDER_ID - 149) * (2**23 + YYMMDDS)

If you wanted, you could specify a template for people to use along these lines:

[Constants]
global $modder_id = ID
global $mod_date = YYMMDD
global $mod_sequence = S
global $mod_id
$mod_id = 2**($modder_id - 149) * (2**23 + $mod_date*10 + $mod_sequence)

Again, this is just theory crafted at this stage and I haven't yet tested this for real in 3DMigoto just yet.

Note that this formula suffers from a Y2K type issue between the years xx84 and xx99 when the encoded YYMMDDS value will itself set the 2**23 bit in the mantissa and the resulting addition will instead attempt to set the 2**24 bit, bumping up the exponent and losing one bit of precision in the sequence number, resulting in odd sequence numbers being unusable for those years and reducing the available modder_ids by 1. Alternate encodings of the date & sequence number would avoid this (e.g. encode it as number of days since some epoch to avoid the holes incurred by using decimal representations of day-of-month and month-of-year, or shift the sequence number to be the high digit and note that it cannot be 8 or 9), but that is sufficiently far into the future that I wouldn't be too worried about this for any current game.

DarkStarSword commented 5 years ago

It is also worth noting that separating out $modder_id from $mod_id and testing both would allow 2x24 bits of precision without the need for a packing formula to fit both in a single variable, though I assume this is intended to fit within an existing infrastructure where this sort of change might not be trivial to implement.

DarkStarSword commented 5 years ago

Additional question, is there a document somewhere in the repo that outlines the INI language? E.g. while trying to figure out variable capabilities, I experimented with keywords (global, pre, post, tried local, etc.). I get how global works now, and that local is just a no prefix variable in a section. I don't really get how pre/post work, if they even still do, or if there are other possible keyword configurations hidden somewhere.

I've been meaning to write up a definitive language reference for some time now - at the moment the documentation is a bit scattered around (the full release notes of any new 3DMigoto release will include details of any new/extended syntax introduced in that version, but I get that isn't the best way to find info about it if you haven't been following along).

Specifically on pre and post - these do work, but their exact meaning is context sensitive. In general pre means "do this action before whatever triggered this command list" and post means do it afterwards, but the meaning of the trigger depends on the command list.

(I try to refer to "pre" and "post" as two separate phases of each command list, but sometimes I may refer to them as though they are separate command lists - the former is how it is supposed to be logically distinguished, but the later is how it is technically implemented in 3DMigoto).

For [ShaderOverride] sections the trigger is the draw call from the game - pre will execute commands prior to the original draw call from the game, while post will execute them afterwards. This is the primary use case of pre and post, and every other application of them is trying to apply similar logic to other contexts, that in some cases have a good fit, and in others it can be a bit of a stretch.

For [Present] pre means before the present call (i.e. the very last things done in each frame - a suitable point to insert UI overlays), and post means after the present call (i.e. the very first things done in a new frame - a suitable place to clear buffers that are no longer needed, reset variables, etc)

For [ClearXXXView] sections pre means before the clear (last chance to copy a buffer the game is discarding), post means afterwards (a chance to modify a buffer the game is clearing).

In all of those cases the context is similar so the meaning is pretty much the same. In other cases, the context is sufficiently different that the meanings are also different:

For [CustomShader] sections, pre and post are run in sequence. In this case the logical thing between the two phases is the injected draw call... only, the draw call is typically injected in one of these phases themselves (and it is valid to have zero or multiple injected draw calls), so it is actually not that meaningful to have a pre and post here. The keywords are still present so that code can be copy & pasted from one section to another without having to add/remove the keywords, given that one of the main use case for explicitly specifying pre and post is to back up and restore a variable or resource bindings and I don't want to have to qualify any example code of these cases with "*unless it's a CustomShader, in which case remove the pre/post keywords".

For [CommandList] and [TextureOverride] sections, the meaning is usually inherited from whatever section ran them (e.g. before and after a draw call if called from a ShaderOverride section). However there is an exception - if the run/checktextureoverride commands themselves were used with a pre/post keyword than the pre and post phases of the called command lists will be run in sequence. Note that since multiple [TextureOverride] sections can be matched by a single checktextureoverride command that all pre sections are run prior to any post sections.

[Key] sections that call command lists depend on the type. type=activate (default) and type=cycle will run the pre phase when they are activated before values specified by the [Key] section have been updated while post will run after values have been updated (but still in the activating frame - this is not deferred with respect to any transitions the key may have triggered since those may be clobbered by other activations). type=hold will run the pre phase when the key is held and the post phase when the key is released.

[Preset] sections have essentially the same meaning as [Key] type=hold - the pre phase is run when the preset is triggered, and the post phase is run once it is no longer being triggered.

[Constants] runs pre and post in sequence, however it should be noted that the d3dx_user.ini file is always the very last config file to be run, so any ini files that wish to do some initialisation after persistent settings have been loaded should use the post phase to insure that all persistent variable assignments have been performed. The pre phase will also not trigger a write of the d3dx_user.ini file if they modify any persistent variables, while the post phase can.

Currently 'pre' and 'post' are problematic (non-intuitive) to use in combination with 'if'. The 'if' command will currently reject these keywords, however they can be applied to commands within the if block and their meaning will be retained from the context of the section they are in. This means that if every command within an if block has the post keyword the entire if block will be executed in the post phase (and indeed will be optimised out of the pre phase entirely in this case), but if there are a mix of phases within the if block it will be run in both phases with the individual contained commands running in just their specified (or implied) phases.

This is something I do wish to change because I don't think it is very intuitive at present. In the future I'd like the 'if' command to itself accept a pre/post keyword and if present all contained commands would be executed in that same phase (all contained pre commands would be run then all contained post commands in sequence). In the meantime, it can be worth splitting complicated if blocks that use these keywords out into separate command lists to keep things clearer (e.g. [Present] pre run = CommandListBeforePresent).

Any command specified without 'pre' or 'post' keywords will implicitly select which phases they apply to. In most cases this will be 'pre', however there are some special commands that default to both, namely run=CommandListXXX, checktextureoverride, if, handling=abort, separation and convergence. The first three because they contain/call other commands that inherit the current phase, the fourth because it's part of flow control and the last two for backwards compatibility reasons (these commands uniquely have special cases when applied to both phases to maintain historical behaviour).

FinalDoom commented 5 years ago

It's been 10+ years since I worked in c++ or graphics, and even then it was basic OpenGL stuff. I think I got confused with the length call thinking of it like javascript or some other (weird) language for some reason. It makes sense what it's doing now looking back at it. Totally makes sense why you're using floats. I guess I thought more would be executed CPU side, but I'm also not really clear on how the program hooks the game exe to begin with, much less where/how things are executed. I remember you going over some of the mechanics of where TextureOverride and such comes in in the GPU rendering order, but that's all very outside my expertise, so it remains fuzzy. That helps clarify a lot though and give me some ideas.

That's an incredibly helpful writeup on the INI. Thank you. It gives me some more ideas maybe how to get messages to display how I wanted them to, understanding pre/post better. I don't know why I didn't even think about actually utilizing the exponent bits. It seems like what I'm going for is leaning toward a little mod template generator sort of thing, if it's going to get mathy. I'm trying to build off the _id logic that's used by a couple people, namely Minazuki, so splitting the mod id and modder id is doable, since not a lot of people follow that paradigm yet, but it makes the conditionals super long in places. Right now, there's an ability to selectively "activate" up to two mods visible on screen at once, for keypress modifications. I've collected a few conflicting mods, though, and am trying to update the old ones in three ways: modernize with variables and so on to cut down duplication and confusion; add in the _id activation; and add in a soft "disable" so you can activate a mod's keybindings, and disable the graphical component completely, so a conflicting mod can take over, and you can easily switch between the two. Part of that is that two slots in _id don't cut it, since there can be up to four models on screen, and conflicts can push the number up further. I settled on 8 as a reasonable compromise--a long if statement comparing ids. More than doubly long if there's a two separate composite ids. In an ideal look, the selective soft disabling can actually be handled by model id more or less, rather than an arbitrary modder-set id. But that'll take some work to get a table of ids up and more to get people to use them. It would be nicer--something like 4 "onscreen" ids, and ctrl+tab to swap between mods that target each id or something. I'll see if I can get something like that working on a small scale. The solution I currently have with 8 activatable onscreen ids is a little buggy anyway, and I'm having trouble figuring out why. Might as well try something else for giggles.

The whole _id thing definitely more of a hack toward an end, considering the framework you've developed is targeted at the graphics portion, but I think it works okay, and I haven't put much thought into coming up with a workable alternative beyond right now writing stuff above. Maybe something else will come to mind as I gain understanding of the various features available. I've also got a workable but UI unfinished "mod manager" that'll display all your Mods/* folder (or whatever is configured in the d3dx.ini) and allow you to get a graphical look at your mods and manipulate the DISABLED state more easily. I'm not happy with it, though, and it was kinda an experiment. This attempt is a bridge between the out of game management and in-game management/configuration.

If you want someone to bounce any documentation off of, or even help write some, from a beginner-intermediate modder standpoint / non graphics programmer side, let me know. I'd love to find a way to help the project.

On Tue, May 21, 2019 at 9:53 PM Ian Munsie notifications@github.com wrote:

Additional question, is there a document somewhere in the repo that outlines the INI language? E.g. while trying to figure out variable capabilities, I experimented with keywords (global, pre, post, tried local, etc.). I get how global works now, and that local is just a no prefix variable in a section. I don't really get how pre/post work, if they even still do, or if there are other possible keyword configurations hidden somewhere.

I've been meaning to write up a definitive language reference for some time now - at the moment the documentation is a bit scattered around (the full release notes of any new 3DMigoto release will include details of any new/extended syntax introduced in that version, but I get that isn't the best way to find info about it if you haven't been following along).

Specifically on pre and post - these do work, but their exact meaning is context sensitive. In general pre means "do this action before whatever triggered this command list" and post means do it afterwards, but the meaning of the trigger depends on the command list.

For [ShaderOverride] sections the trigger is the draw call from the game - pre will execute commands prior to the original draw call from the game, while post will execute them afterwards. This is the primary use case of pre and post, and every other application of them is trying to apply similar logic to other contexts, that in some cases have a good fit, and in others it can be a bit of a stretch.

For [Present] pre means before the present call (i.e. the very last things done in each frame - a suitable point to insert UI overlays), and post means after the present call (i.e. the very first things done in a new frame - a suitable place to clear buffers that are no longer needed, reset variables, etc)

For [ClearXXXView] sections pre means before the clear, post means afterwards.

In all of those cases the context is similar so the meaning is pretty much the same. In other cases, the context is sufficiently different that the meanings are also different:

For [CustomShader] sections, pre and post are run in sequence. In this case the logical thing between these two sections is the injected draw call... only, the draw call is typically injected in one of these command lists themselves, so it is actually not that meaningful to have a pre and post here. The keywords are still present so that code can be copy & pasted from one section to another without having to add/remove the keywords, given that the main use case for explicitly specifying pre and post is to back up and restore a variable or resource bindings and I don't want to have to prefix any example code of these cases with "*unless it's a CustomShader, in which case remove the pre/post keywords".

For [CommandList] and [TextureOverride] sections, the meaning is usually inherited from whatever section ran them (e.g. before and after a draw call if called from a ShaderOverride section). However there is an exception - if the run/checktextureoverride commands themselves were used with a pre/post keyword than the pre and post phases of the called command lists will be run in sequence. Note that since multiple [TextureOverride] sections can be matched by a single checktextureoverride command that all pre sections are run prior to any post sections.

[Key] and [Preset] sections that call command lists depend on the type. type=activate (default) and type=cycle will run the pre phase when they are activated before values specified by the [Key/Preset] section have been updated while post will run after values have been updated (but still in the activating frame - this is not deferred with respect to any transitions the key may have triggered since those may be clobbered by other activations). type=hold will run the pre phase when the key is held / preset is triggered and the post phase when the key is released / preset is no longer being triggered.

[Constants] runs pre and post in sequence, however it should be noted that the d3dx_user.ini file is always the very last config file to be run, so any ini files that wish to do some initialisation after persistent settings have been loaded should use the post command list to insure that all persistent variable assignments have been performed. The pre command lists will also not trigger a write of the d3dx_user.ini file if they modify any persistent variables, while the post command lists can.

Currently 'pre' and 'post' are problematic (non-intuitive) to use in combination with 'if'. The 'if' command will currently reject these keywords, however they can be applied to commands within the if block and their meaning will be retained from the context of the section they are in. This means that if every command within an if block has the post keyword the entire if block will be executed in the post phase (and indeed will be optimised out of the pre phase entirely in this case), but if there are a mix of phases within the if block it will be run in both phases with the individual contained commands running in just their specified (or implied) phases.

This is something I do wish to change because I don't think it is very intuitive at present. In the future I'd like the 'if' command to itself accept a pre/post keyword and if present all contained commands would be executed in that same phase (all contained pre commands would be run then all contained post commands in sequence). In the meantime, it can be worth splitting complicated if blocks out into separate command lists to keep things clearer (e.g. [Present] pre run = CommandListBeforePresent).

Any command specified without 'pre' or 'post' keywords will implicitly select which command lists they apply to. In most cases this will be 'pre', however there are some special commands that default to both, namely run=CommandListXXX, checktextureoverride, if, handling=abort, separation and convergence. The first three because they contain/call other commands that inherit the current phase, the fourth because it's part of flow control and the last two for historical / backwards compatibility reasons.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bo3b/3Dmigoto/issues/111?email_source=notifications&email_token=AAFFN2JWT35HKREWIFJDHQ3PWSRSLA5CNFSM4HOHLXDKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV5VHJA#issuecomment-494621604, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFFN2JL2BRNI2Z3BBX7OI3PWSRSLANCNFSM4HOHLXDA .

DarkStarSword commented 5 years ago

In terms of where things are executed - there is a fairly clear division with the command lists being executed on the CPU (part of which is issuing commands to the GPU, hence the name) and shaders being executed on the GPU, but a lot of the traditional use case for the command list has been to ultimately pass values to the GPU so it has been a priority for that to be as seamless as possible, especially considering that a lot of modders we support are copying & pasting examples and not necessarily understanding the full nuances involved (and traditionally it has been fairly rare for modders to require large values that might not fit in 24 bits of precision). That is changing though - the addition of named variables and the 'if' block to the command list (and more generally extending 3DMigoto to support costume modding) has meant that we can now do a lot more on the CPU side than we used to.

The idea of adding a UI manager for the mods folder has been on my radar as well - I've been trying to encourage @helifax to port his config UI from his OpenGL modding tool to 3DMigoto to allow modders to edit shaders without alt+tabbing out (which can be a royal pain for 3D Vision modding) and once that is in extending it to allow end users to toggle mods on and off would be nice to have. We could even look at adding more flexible configuration at that time. That said, I don't think this is coming from either of us any time soon as both Helifax and I have much more pressing matters to work on, but I'd certainly be interested in anything you may have to contribute here.

BTW is there any support for any constructs from other languages that would be of particular use to you? Things like declaring and indexing arrays and passing arguments when calling functions and returning values are on my TODO list, but are lowish priority - but I'm always interested in knowing if there is anything I should bump up a bit on my list, or anything I may not have thought of?

You should definitely join the 3D Vision discord - 3DMigoto has a channel on there (there's also a thread on the Geforce forums): https://discord.gg/tXHYhNK

FinalDoom commented 5 years ago

Cool, joined.

I think arrays are about all that comes to mind immediately as useful, and even then it's just a mild convenience -- one extendable variable instead of multiple hardcoded. I'll give it some thought as I'm toying with things though. Other data types again are also just convenience, and as you've shown, there's probably reasonable ways to work things into a float package. It might be nice to be able to specify variables as hex, but that makes more sense with an int type, and doesn't buy any more space with whatever data type (thinking in terms of the id thing again).

Is @helifax config tool source open? Maybe I could have a look at that to get more familiar with the code.

DarkStarSword commented 5 years ago

Is @helifax config tool source open?

Yes and no - there is a public version available ( https://github.com/helifax/OGL-3DVision-Wrapper ), but my understanding is that is quite out of date by now (and wouldn't include any of the newer config UI). I don't really want to speak for him, but I don't think there was any fundamental reason the newer code isn't available, beyond that it is still a work in progress and he didn't want people bothering him about something that wasn't ready yet (correct me if I have any of that wrong Helifax). I think he may have given me access to a private repo with newer code at one point... but have actually forgotten the details of that and can't seem to find it now.