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

Top Gear - graphical glitches with sprite occlusion on the road #303

Closed
Kopert opened this issue Nov 14, 2021 · 14 comments
Closed

Top Gear - graphical glitches with sprite occlusion on the road #303

Kopert opened this issue Nov 14, 2021 · 14 comments

Comments

@Kopert
Copy link

Kopert commented Nov 14, 2021

I don't know if sprite occlusion is the correct term, I don't know how it is actually implemented. Anyway:

Core: SNES_20210713.rbf
ROM: NO-Intro, Top Gear (USA).sfc, MD5 e5040a079ff03eb93747424af9be75f4

Issue: Sometimes sprites still show up when they're supposed to be occluded by the road, for example, when going downhill.

Here are two images that show the problem:

IMG_20211114_154448
IMG_20211114_153849
On these images, taken near the last turn on the first level (Las Vegas, U.S.A), you can see a left-turn signal that should be hidden by the road. I've noticed this on other levels with other sprites, including cars.

I do not have an actual SNES to test this one, but I have checked this video taken from the actual console and I've tested it on the bsnes_mercury_accuracy RetroArch core, and the issue does not appear on either case.

@Kopert
Copy link
Author

Kopert commented Nov 21, 2021

I don't know if this helps, but I tested SNES_20210713.rbf, SNES_20210620.rbf, SNES_20210225.rbf, and SNES_20201106.rbf (just a random sampling) and I could reproduce the issue in all of these.

@paulb-nl
Copy link
Contributor

I was able to reproduce the issue and I can confirm that it doesn't happen on a real SNES.

I have been looking into how the game hides parts of the sprite but I don't understand it yet.

@Kopert
Copy link
Author

Kopert commented Nov 22, 2021

I was able to reproduce the issue and I can confirm that it doesn't happen on a real SNES.

I have been looking into how the game hides parts of the sprite but I don't understand it yet.

Thank you for taking your time and confirming the issue. Let me know if there is any way I can help.

@paulb-nl
Copy link
Contributor

The game hides parts of sprites by adding many off-screen sprites that still take up time to fetch so that there is no time left to fetch the sprites which should be hidden.
top_gear_sprites

These off-screen sprites are 16x16 with X position -256(or 256) so these should take 4 pixel cycles to fetch but the core only takes 2 pixel cycles to fetch these sprites.

This trace shows sprite 46 takes 4 pixel cycles to fetch which is correct because it is a 16x16 sprite with X position 97. Sprite 47-51 are 16x16 off-screen sprites but only take 2 pixel cycles to fetch which should be 4 cycles.
trace_top_gear_sprites

Here is a save state for Mesen-S:
Top Gear (U)_2.mss.zip

@sroccaserra
Copy link

Nice, thank you for this info!

@paulb-nl
Copy link
Contributor

For reference, here is a capture of a SNES. It looks the same as with the save state of Mesen-S. The car to the left of the white car is what causes the missing sprite lines in the white car.
topgear_sprite_hidden

@paulb-nl
Copy link
Contributor

There is a check here that skips to the next sprite if the next tile is off-screen. The or (TILE_X + 8) >= 256 part.

if CUR_TILES_CNT = W(5 downto 3) or (TILE_X + 8) >= 256 then

@srg320 Do you remember why this check is done? It doesn't seem to match a real SNES with Top Gear.

If I remove or (TILE_X + 8) >= 256 then the result is like a real SNES:
20211126_115620-Top Gear (U)x

@Kopert Can you test this build with other games?
SNES_sprite.rbf.zip

@Kopert
Copy link
Author

Kopert commented Nov 26, 2021

@Kopert Can you test this build with other games? SNES_sprite.rbf.zip

Thank you for the build, @paulb-nl. I tested it on about a dozen games keeping my eyes up anything that looked out of place and couldn't find anything.

Click to see list of games tested on 2021/11/26 with SNES_Sprite.rbf

My main concern is that the issue with Top Gear was very hard to perceive in the first place, so there's a chance that any issues your changes would introduce would easily escape my notice. Ideally we'd know why the or (TILE_X + 8) >= 256 was added in the first place. It might have been just some optimization that does not reflect real hardware, but it might have been added to fix some other issue.

I will continue using your build as my daily driver so if anything out of place pops up I'll let you know. Thanks again!

@paulb-nl
Copy link
Contributor

paulb-nl commented Dec 2, 2021

The sprite test rom I made does show that the SNES only fetches the visible tiles so that's why that TILE_X+8 check is there.

Only when a sprite is set to X position -256/256 then it does fetch all the tiles even though the sprite is completely off-screen.

@Kopert
Copy link
Author

Kopert commented Dec 2, 2021

The sprite test rom I made does show that the SNES only fetches the visible tiles so that's why that TILE_X+8 check is there.

Only when a sprite is set to X position -256/256 then it does fetch all the tiles even though the sprite is completely off-screen.

I'm out of my depth here, but would that be just a case of replacing (TILE_X + 8) >= 256 with (TILE_X + 8) > 256 then?

@paulb-nl
Copy link
Contributor

paulb-nl commented Dec 4, 2021

That doesn't work because TILE_X equals 256 on the first tile of a sprite with X position 256. 256+8=264.

Also it would not skip the next tile of a sprite if the next tile starts at 256.

@Kopert
Copy link
Author

Kopert commented Dec 10, 2021

I just tested SNES_unstable_20211210_c8c9, which includes 3c622c1 (fixes #306), and I can confirm the Top Gear issue is fixed on this build. Thanks, everyone, for your time!

Should I close the issue, or is there need for additional testing or development?

@sorgelig
Copy link
Member

better to wait for release, then test and close if fixed.

@Kopert
Copy link
Author

Kopert commented Dec 14, 2021

Tested with release 21.12.14. Could not reproduce the issue anymore. Thanks for the fix!

@Kopert Kopert closed this as completed Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants