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

Correct some of immediate vertex handling #15978

Merged
merged 8 commits into from
Sep 7, 2022

Conversation

unknownbrackets
Copy link
Collaborator

I don't have #7459 or see a frame dump for it, so I couldn't validate if this breaks it.

Anyway, this makes the GE debugger treat immediate prims as prims for stepping purposes (some games send zeros on reset, so hopefully it won't change dump counts much or cause annoyance.) It doesn't prevent them, but at least you'll hit them.

It also handles their flags and various prim types much more accurately. I have a test that draws all the types and it now works fine, at least in softgpu and Vulkan (although Vulkan doesn't match the dither, of course.)

-[Unknown]

Comment on lines 2393 to 2394
v.x = ((gstate.imm_vscx & 0xFFFF) - 0x8000) / 16.0f;
v.y = ((gstate.imm_vscy & 0xFFFF) - 0x8000) / 16.0f;
Copy link
Owner

@hrydgard hrydgard Sep 6, 2022

Choose a reason for hiding this comment

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

Gonna have to be

((int)(gstate.imm_vscx & 0xFFFF) - 0x8000) / 16.0f;

Otherwise we'll have an unsigned value after subtraction if the result is negative, which will be converted to floating point as a huge number. Not new though, hm.

Copy link
Owner

@hrydgard hrydgard Sep 6, 2022

Choose a reason for hiding this comment

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

Are you sure that offsetX and offsetY are not involved? Without them, the rectangle in ThrillVille becomes -240, -136 to 240, 136 in through mode coordinates, instead of the expected and working 0,0 to 480,272.

Of course in ThrillVille's case they're just 0, so maybe there is no -0x8000 subtraction, or the offset comes from elsewhere...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh right. All my test verts were positive, oops, didn't think about negative hard... had just found the offset seemed fixed.

Well, in my test I tried varying the offset registers and the primitives did not translate. To target a specific X I just had to send in the register: ((2048 + v[i].x) << 4). Even 0x00FF0000 | ((2048 + v[i].x) << 4) was the same.

This worked even when I used sceGuViewport(0, 0, 1, 1); sceGuOffset(0, 0); (which previously made graphics not draw at all in PPSSPP.) The values of those registers make no difference to what gets drawn.

Can you attach a frame dump including ThrillVille's case? Maybe there's some other register that affects it.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

thrillville title ULES00889_0002.zip

Here's a framedump on the title screen, where it only clears a quarter of the screen.
Playing back the framedump crashes in FramebufferMaanger.cpp on line 813 for some reason, easily fixed by adding currentRenderVfb_ != nullptr to the condition (should probably commit that).

Copy link
Collaborator Author

@unknownbrackets unknownbrackets Sep 7, 2022

Choose a reason for hiding this comment

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

Interesting. I found some more issues looking at this dump, gonna dig into it a bit more in just a bit. Pushed some updates (including two fixes for crashes.)

My immediate thought was maybe it was clear - which it isn't. But clear does apply, prevents texturing, and masks rgb/s/z per its flags. So that's good to have confirmed. Lots of state that could be checked here, sigh.

What's also weird is softgpu is skipping the first draw (even in the TL quadrant), and I don't know why yet. Nevermind, fixed.

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, that's fun. It depends on the current vertextype register. If that has throughmode set, then offset is fixed. Otherwise, offset is applied. This probably also explains why I couldn't get fog to apply.

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, investigated the other flags (confirmed we can use this to directly set the 8-bit fog factor, which was actually my original goal investigating this, because the softgpu fog formula is still inaccurate and the floats got in the way.)

Here's how it compares now with that dump vs PSP (__testcompare output, transparent pixels highlight inaccuracies):
__testcompare.png

Still off in a bunch of places, but the inaccuracy is fairly small. I think it's mostly related to the triangle loop now.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

Cool!

This immediate stuff is one pretty wacky feature. Wonder whether it was only intended for early hardware debugging or it was determined that it was confusing and not a performance win, and that's why it never was documented (presumably, due to the low usage).

This allows for lines, points, textures, and similar things.  Also
corrects offset handling.  Still some flags on VAP that seemingly don't
work, and this doesn't consider the texture flag on it.
And respect the other flags that I can reproduce working in a test.
I can't seem to get the fog to work at all, or the shading mode, or the
secondary color.  Maybe depends on other flags or bits in other regs...
These parameters are a real shame, was so clean before...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants