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

[graphics] MERC #1124

Merged
merged 16 commits into from
Feb 4, 2022
Merged

[graphics] MERC #1124

merged 16 commits into from
Feb 4, 2022

Conversation

water111
Copy link
Collaborator

Lots of weird bugs still, but it is recognizable as Jak and Daxter.

Don't look too closely at MercProgram.cpp... it's gross.

@water111
Copy link
Collaborator Author

water111 commented Feb 1, 2022

MERC Bugs

Textures failing test

It's particularly obvious on crates:
image
but also happens on various platforms/precursor doors.

Fix

This was a bunch of things:

  • We used > when we should have used >= for alpha test, incorrectly failing pixels when alpha test is disabled and alpha is 0.
  • The merc VU program can disable the alpha test and there was a pipelining bug in the branch logic, meaning we used a bogus value to decide when to include the tag.
  • When running merc with data meant for generic, the alpha test is not set properly. For now, we just have to live with this.

The crates turn off alpha test (otherwise they would correctly fail)
image

The rings above the FJ temple (lo-res version) required the pipelining to be right, otherwise they would disable alpha test incorrectly:
image
(the previous strip turns it off, it should be turned back on here).

Jak is Transparent

image
Happens reliably at the village2 checkpoint. Just walk forward slightly. Jak's hair is sometimes transparent too, but this might be okay because we're rendering his hair with the wrong renderer.

Fix

direct basic shader alpha was wrong.

Merc and tfrag dirt don't seem to line up

Jak's feet are on the ground correctly, but the "dirt" is above the ground:
image

This could be a tfrag issue instead of a merc issue, I'm not sure.

Fix

The "dirt" is actually slightly above the ground, but it shouldn't have been writing the depth buffer due to the way they set up alpha test. It looks like I started writing this (see alpha_hack_to_disable_z_write) but I guess I forgot to finish it?

image

Texture "flashing"

Hard to capture a screenshot, but the fisherman's boat tends to flash violently.

Fix

wasn't flushing renderer...

Near objects (clipping?) sometimes positioned wrong.

image

The normal merc renderer can clip using a mode called "merc prime". However, it can't always clip, evidently if an edge is too long, as seen on-screen I think. (side note: this is why all the skelgroups have a longest-edge thing).
The check longest edge check appears to work (has reasonable numbers, and eventually decides things are too close). But, if you get right near the limit, merc will have some weird clipping failures.
An easy way to trigger this is the first cutscene when you talk to keira, daxter walks toward the camera.

Another good spot is to walk a bit forward
image
image

Sky Garbage

image

Sometimes the sky is filled with garbage. This is merc triangles that are very far in the distance. This might be the same as the problem above but the object ends up partially behind the camera.

Merc sends adgifs that disable z test but probably shouldn't

Some number of merc adgifs disable the z test. I think this probably shouldn't happen.

Fix

Was the pipeline bug

Waterfall in lo-res beach level transform one frame behind

It looks like the waterfall part of the beach level is using the transformation from the previous frame. If you pan the camera quickly it lags behind.

Fix

Was the flush bug.

@water111
Copy link
Collaborator Author

water111 commented Feb 2, 2022

With those bug fixes, the only remaining issue is the clipping stuff. It looks like any time mscal 17 or 32 is used ("merc prime") the output is garbage. The decision to use normal/prime appears to be correct, and the decision to go from prime -> generic is also reasonable looking. So I don't think we're accidentally using prime when we should be using generic.

Interestingly, if you use prime in cases where you don't need it, it is bad in a similar way. I'd expect this to work (as the guard band check that picks merc prime over normal is conservative)

@water111 water111 marked this pull request as ready for review February 4, 2022 00:10
@water111
Copy link
Collaborator Author

water111 commented Feb 4, 2022

Merc is now working correctly! I've also set it up to try rendering things that would normally be rendered by other renderers, so there are still some graphical glitches caused by this. In particular, stuff close to the camera can have clipping disasters and stuff rendered normally with generic may have bad blending/alpha settings.

@coveralls
Copy link

coveralls commented Feb 4, 2022

Pull Request Test Coverage Report for Build 1793096860

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 42 of 4072 (1.03%) changed or added relevant lines in 16 files are covered.
  • 30 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-2.3%) to 47.193%

Changes Missing Coverage Covered Lines Changed/Added Lines %
decompiler/VuDisasm/VuInstruction.h 0 1 0.0%
game/graphics/opengl_renderer/tfrag/tfrag_common.cpp 0 1 0.0%
common/dma/dma.cpp 0 2 0.0%
decompiler/VuDisasm/VuInstruction.cpp 0 2 0.0%
decompiler/analysis/mips2c.cpp 0 5 0.0%
game/graphics/opengl_renderer/DirectRenderer.cpp 0 13 0.0%
game/graphics/opengl_renderer/OpenGLRenderer.cpp 0 28 0.0%
common/dma/dma.h 0 33 0.0%
game/mips2c/mips2c_private.h 0 46 0.0%
game/graphics/opengl_renderer/MercRenderer.h 0 175 0.0%
Files with Coverage Reduction New Missed Lines %
common/dma/dma.h 1 0.0%
game/mips2c/functions/draw_string.cpp 1 0.78%
game/mips2c/functions/sparticle.cpp 1 2.59%
common/dma/dma.cpp 2 0.0%
../../../../../usr/lib/gcc/x86_64-linux-gnu/9/include/emmintrin.h 2 0.0%
game/graphics/opengl_renderer/DirectRenderer.cpp 3 0.0%
game/mips2c/mips2c_private.h 3 4.43%
../../../../../usr/lib/gcc/x86_64-linux-gnu/9/include/xmmintrin.h 17 0.0%
Totals Coverage Status
Change from base Build 1775933338: -2.3%
Covered Lines: 41221
Relevant Lines: 87346

💛 - Coveralls

@water111 water111 merged commit 2342b6f into master Feb 4, 2022
@water111 water111 deleted the w/merc branch February 4, 2022 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants