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

Fix Worms Armageddon one pixel garbage line under names and menu items #1847

Closed
wants to merge 1 commit into from

Conversation

gizmo98
Copy link
Contributor

@gizmo98 gizmo98 commented Jun 26, 2018

There is a one pixel garbage line under every menu item and under every worm name . If gSP.bgImage.height is imageH - 1 there is no garbage. I have tested this fix with multiple games and have not seen any regression.

I also replaced imageW modulo operation with & 0xFFFFFFFE.

There is no garbage line if gSP.bgImage.height is always imageH - 1. I
also replaced imageW modulo operation with & 0xFFFFFFFE.
gSP.bgImage.width = imageW - imageW%2;
gSP.bgImage.height = imageH - imageH%2;
gSP.bgImage.width = imageW & 0xFFFFFFFE;
gSP.bgImage.height = imageH - 1;
Copy link
Owner

Choose a reason for hiding this comment

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

I suppose that imageH - imageH%2 was done for purpose.
imageH & 0xFFFFFFFE would cause no questions.
I'm afraid that imageH - 1 will cause regressions. I need to test it first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gonetz
There can be regressions but i have seen none. I have tested beetle adventure racing, sm64, mario tennis, smash bros, paper mario, yoshis story, mischief makers, bomberman and many other games, re2, doom, mario kart and goldeneye.

@olivieryuyu
Copy link

Does it work ok in lle?

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jun 27, 2018

@olivieryuyu
I don't know. But a garbage pixel line at the bottom of a texture with sometimes changing pixel color looks suspicious.

Comparison:
At the moment
org1
org2
org3

With fix
fix1
fix2
fix3

@olivieryuyu
Copy link

this is a HLE issue as in LLE not such an issue.

I am working on the ucode (slowly but surely, i will take my time on this) but from there a proper fix should be found for all games with S2DEX errors.

@theboy181
Copy link

I wonder if this is related to the issue I’m seeing in F-zero with the mph numbers showing up at the bottom of the texture.

@theboy181
Copy link

Is there a binary available so I can test the changes?

@DonelBueno
Copy link

DonelBueno commented Jun 29, 2018

Mario Tennis pause menu also has a garbage line. I'm sure I've seen this in other games too, but Mario Tennis is the only one that comes to my mind.

@tony971
Copy link
Contributor

tony971 commented Jun 30, 2018

Mario Kart 64 has a garbage line to the right of the split screen race finish screen (on master. Dont know if this PR affects it.)

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jul 1, 2018

@theboy181
I have only a armv7 mp64 binary.

@olivieryuyu @gonetz
According to SDEX.h imageW is 8byte aligned (https://github.com/gonetz/GLideN64/blob/master/src/uCodes/S2DEX.h#L22). imageH is not https://github.com/gonetz/GLideN64/blob/master/src/uCodes/S2DEX.h#L29. So if imageSiz is G_IM_SIZ_32b and imageW is uneven we need one more texel to get a even number of texels. The modulo operation takes care we have a even number of texels. (Do we need to clamp the extra texel?)

ALL textures must Align to 64bit boundries in each row.
eg. a 32bit texture MUST use multiples of 2 texels, while a 4bit texture MUST use multiples of 16 texels.
http://fgfc.ddns.net/PerfectGold/Textures.htm

Quite interesting. Do we ned more than one fill up texel if imageSiz is G_IM_SIZ_16b, G_IM_SIZ_8b or G_IM_SIZ_4b?

I can write my PR a little bit different to get the same result:
gSP.bgImage.width = (imageW - 1 ) + (imageW - 1 )%2;
gSP.bgImage.height = imageH - 1;

@olivieryuyu
Copy link

olivieryuyu commented Jul 1, 2018

Interesting!

Do we need to clamp the extra texel?

See gSPObjRenderMode in manual

I invite also you have a look at some other games with S2DEX issues: #936 and #133

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jul 3, 2018

I have found more information:
https://level42.ca/projects/ultra64/Documentation/man/pro-man/pro25/25-05.html

BG image's width, imageW must be aligned to 8 bytes. Since the actual values used for imageW and imageH are in (u10.2) format, the values to be assigned must be multiplied by 4. The following chart shows the imageW's value constraints, taking (u10.2) format into consideration and multiplying by 4. There is no need to align imageH values.

  • When G_IM_SIZ_4b: | imageW is a multiple of 64
  • When G_IM_SIZ_8b: | imageW is a multiple of 32
  • When G_IM_SIZ_16b: | imageW is a multiple of 16
  • When G_IM_SIZ_32b: | W is a multiple of 8

So if modulo operation is there to calculate the right imageW it is not necessary to correct imageH.

@olivieryuyu
Copy link

this issue did not exit in Sep 2017. It is a new one. Seems we are turning around an issue...

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jul 20, 2018

@olivieryuyu if it is a regression i could try to bisect the commit which changes behaviour.

@olivieryuyu
Copy link

it was not here in September 2017. I guess something happened after.

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jul 29, 2018

Reverting feb0e53 but keeping 8940622 fixes it.

@gizmo98
Copy link
Contributor Author

gizmo98 commented Jul 31, 2018

I will close this for now. I get equal or better results if i revert above mentioned commit and modify ObjCoordinates creation.

@gizmo98 gizmo98 closed this Jul 31, 2018
@gizmo98 gizmo98 mentioned this pull request Jul 31, 2018
@gizmo98 gizmo98 deleted the worms-fix branch August 4, 2018 17:59
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.

None yet

6 participants