Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[F3D] Combiner Overflow Preview Fix 2 #354

Merged
merged 2 commits into from
Jun 29, 2024

Conversation

jesusyoshi54
Copy link
Collaborator

The Shade input on C was causing overflow because it was registering as slightly above one due to float imprecision. To create a proper distance between 255 inputs vs 256 inputs, I made the 1.0 value 1.0001 and the overflow now occurs at 1.0001. This should make it so that only Combined values can overflow, as they're the only ones that can be >255 on the C input (besides for YUV K5 but fast64 does not allow K5 values above 1.0).
Fixes #348

@Dragorn421
Copy link
Contributor

Why

made the 1.0 value 1.0001

?
(you are speaking of the 1 input to the combiner right?)

it sounds like that just makes it more likely for 1 to wrongly wrap to -1 on the C combiner inputs

@Dragorn421
Copy link
Contributor

Testing wise, unlike the previous attempt OoT's imported kokiri forest and deku tree are fine :)

@jesusyoshi54
Copy link
Collaborator Author

Why

made the 1.0 value 1.0001

? (you are speaking of the 1 input to the combiner right?)

it sounds like that just makes it more likely for 1 to wrongly wrap to -1 on the C combiner inputs

It makes it so that it correctly wraps with an input of 1.0 on C. There is a reason why C by default doesn't allow an input of 1.0 but every other input does. 1.0001 represents 256 in a float number system and we need it to actually wrap on 1.0001 to get the weird CC behavior or else it won't mirror the actual CC. Here is a test image, with 1 cycle passing 1.0, and the top left is shade*combined, and the top right is combined*shade. The thing that is causing the issue here is the overflow of 1.0, which makes it overflow once shade reaches a value less than 0x80. I can't do anything about the way vertex colors interpolate, but other than that this matches.
image

Here is a code example to make it more obvious, the equations in python were shared by Tharo when this was first discussed.

def ext(x):
    return x | -(x & 0x100)

def spext(x):
    return (x | ~0x1FF) if ((x & 0x180) == 0x180) else (x & 0x1FF)

def cc_calc(a,b,c,d):
    a = spext(a)
    b = spext(b)
    c = ext(c)
    d = spext(d) << 8
    combined = (a - b) * c + d
    return (combined + 0x80) & 0x1FFFF

and then the test case. Overflow occurs at 384 (1.5) on the final output.

# first cycle examples
>>> cc_calc(0, 0, 0, 256) # (0 -0) *0 + 1.0
65664 # 256
>>> cc_calc(256, 255, 255, 255) # (1 - prim) * tex0 + prim
65663 # 256

# now with C input, we can see that we get overflow with A inputs < 0.5
>>> cc_calc(0x80, 0, 65663>>8, 0)>>8
384
>>> cc_calc(0x81, 0, 65663>>8, 0)>>8
383
>>> cc_calc(0x7F, 0, 65663>>8, 0)>>8
385
# and as a comparison, here is the same test with combined on A
>>> cc_calc(65663>>8, 0, 0x7f, 0)>>8
127
>>> cc_calc(65663>>8, 0, 0x80, 0)>>8
128
>>> cc_calc(65663>>8, 0, 0x81, 0)>>8
129

So yeah, 1.0 exactly on C is doing weird stuff. Here is an image with 1.0 passed

Copy link
Contributor

@Dragorn421 Dragorn421 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I get it, didn't know about this, that's weird
thanks for the details

@Dragorn421 Dragorn421 added merge soon Will be merged in a few days at most if nothing else comes up f3d Has to do with the "f3d" code common to all games labels Jun 22, 2024
@Dragorn421 Dragorn421 merged commit 7da4b7e into Fast-64:main Jun 29, 2024
1 check passed
@jesusyoshi54 jesusyoshi54 deleted the mat_clamp_fix branch July 28, 2024 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f3d Has to do with the "f3d" code common to all games merge soon Will be merged in a few days at most if nothing else comes up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve combiner overflow behavior
2 participants