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

Software: Handle texture wrapping more accurately #9921

Merged
merged 2 commits into from Jul 25, 2021

Conversation

Pokechu22
Copy link
Contributor

@Pokechu22 Pokechu22 commented Jul 20, 2021

See https://bugs.dolphin-emu.org/issues/12408. When texture wrapping is enabled, the texture size should be a power of two per libogc; yagcd mentions (near bp register 0x80) that repeat and mirror "[require] the texture size to be a power of two. (wrapping is implemented by a logical AND (SIZE-1))".

Although I initially targeted this at the the non-power-of-2 issue, it also impacts the software renderer when wrapping textures in negative coordinates (https://dolp.in/i11834/6 is an example). This is probably due to the modulus operator % returning negative numbers for negative inputs (e.g. -1 % 4 is -1), while using & gives positive values (e.g. -1 & (4-1) is 3).

I haven't implemented the non-power-of-2 stuff in the hardware backends, as the graphics APIs themselves handle texture wrapping in the sampling process. Dolphin could handle it manually in the pixel shader, but I suspect there would be a performance cost to it for something that basically no games do (though I haven't tried it and the performance impact may be minimal or it might even be faster due to allowing silly behavior for non-power-of-2 sizes). One other possible complication to implementing it on the hardware backends is custom textures, as those might not be a power-of-2 size and using a bitwise and might also get rid of fractional parts of coordinates which would break higher resolution textures (though these too may not be issues).

I did also find that setting the wrap mode to 3 (which is invalid) seems to behave the same as setting it to 0 (clamp). Dolphin previously treated it the same as 1 (repeat) instead; I've fixed that for both the hardware and software backends.

@Pokechu22 Pokechu22 force-pushed the non-power-of-2-wrap branch 2 times, most recently from bca5b9d to a839ab4 Compare July 20, 2021 06:09
@Pokechu22

This comment has been minimized.

coord = coord % (imageSize + 1);
coord = (coord < 0) ? imageSize + coord : coord;
// Per YAGCD's info on TX_SETMODE1_I0 (et al.), mirror "requires the texture size to be a power
// of two. (wrapping is implemented by a logical AND (SIZE-1))". So though this doesn't wrap
// nicely for non-power-of-2 sizes, that's how hardware does it.
coord = coord & (image_size - 1);
coord = (coord < 0) ? image_size - 1 + coord : coord;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The (coord < 0) case is no longer needed, as the bitwise and will always result in a positive value.

I suspect that this is also responsible for all of the rendering differences I mentioned above (and initially incorrectly attributed to size + 1), which should also bring the software renderer in line with the hardware renderers.

@Pokechu22 Pokechu22 force-pushed the non-power-of-2-wrap branch 2 times, most recently from d85f035 to f53ddfa Compare July 20, 2021 19:56
@Pokechu22 Pokechu22 changed the title Software: Accurately reflect console behavior when using texture wrapping with a non-power-of-2 size Software: Handle texture wrapping more accurately Jul 20, 2021
@Pokechu22
Copy link
Contributor Author

Hardware test results:

These were done by editing object 306 in the hardware fifo player, editing BPMEM_TX_SETMODE0 at offset 00119e80. The data there by default is 6180018195, where the 5 at the end indicates that both U and V wrapping is set to 1 (repeat). Since only the texture's width is a power of 2, we can set U to 0 for clamp without any differences. Thus, I tested the v wrapping with 6180018190 (Clamp), 6180018194 (Repeat), 6180018198 (Mirror), and 618001819c (Invalid). I also edited the texture's height in BPMEM_TX_SETIMAGE0 at offset 00119e8a.

Height = 191

Leave 00119e8a unchanged as 618852f9ff.

Mode Hardware Dolphin
Clamp (0) Screenshot 2021-07-20 10-42-32_0 OHBCHB_2021-07-20_12-42-28_0
Repeat (1) Screenshot 2021-07-20 10-43-08_4 OHBCHB_2021-07-20_12-42-12_4
Mirror (2) Screenshot 2021-07-20 10-43-56_8 OHBCHB_2021-07-20_12-42-45_8
Invalid (3) Screenshot 2021-07-20 10-44-50_c OHBCHB_2021-07-20_12-43-03_c
Height = 192

Change 00119e8a to 618852fdff.

Mode Hardware Dolphin
Clamp (0) Screenshot 2021-07-20 10-50-24_r0 OHBCHB_2021-07-20_12-43-30_r0
Repeat (1) Screenshot 2021-07-20 10-50-51_r4 OHBCHB_2021-07-20_12-43-38_r4
Mirror (2) Screenshot 2021-07-20 10-51-27_r8 OHBCHB_2021-07-20_12-43-47_r8
Invalid (3) Screenshot 2021-07-20 10-51-53_rc OHBCHB_2021-07-20_12-43-58_rc
Height = 96

Change 00119e8a to 61885181ff.

Mode Hardware Dolphin
Clamp (0) Screenshot 2021-07-20 12-26-55_s0 OHBCHB_2021-07-20_12-44-42_s0
Repeat (1) Screenshot 2021-07-20 12-27-19_s4* OHBCHB_2021-07-20_12-44-49_s4
Mirror (2) Screenshot 2021-07-20 12-28-12_s8 OHBCHB_2021-07-20_12-44-59_s8
Invalid (3) Screenshot 2021-07-20 12-28-30_sc OHBCHB_2021-07-20_12-45-09_sc

*Repeat had some unusual behavior where one row changed each time it drew. Here's another image of it. The actual texture being drawn seems the same, apart from the places where no texture is drawn at all, so I think the wrapping logic is correct.

It's important to note that the original issue I was trying to fix was from a mod that unintentionally had texture wrapping enabled when it should have been disabled; the buggy-looking wrapped appearance is what they should have gotten in Dolphin (and got when they later tested it on actual hardware).

@Pokechu22 Pokechu22 marked this pull request as ready for review July 20, 2021 23:34
@Pokechu22
Copy link
Contributor Author

Notable differences (apart from just texture shifting):

This fixes various texture offsetting issues with negative texture coordinates (bringing the software renderer in line with the hardware renderers).  It also handles the invalid wrap mode accurately (as was done for the hardware renderers in the previous commit).  Lastly, it handles wrapping with non-power-of-2 texture sizes in a hardware-accurate way (which is somewhat broken looking, as games aren't supposed to use wrapping with non-power-of-2 sizes); this has not been done for the hardware renderers.
@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • aeon-charge-attack on sw-lin-mesa: diff
  • bk-tev on sw-lin-mesa: diff
  • burnout2-vehicletextures on sw-lin-mesa: diff
  • chibi-robo-fastdepth on sw-lin-mesa: diff
  • chibi-robo-zfighting on sw-lin-mesa: diff
  • custom-brawl-char on sw-lin-mesa: diff
  • dbz-depth on sw-lin-mesa: diff
  • DKCR-Char on sw-lin-mesa: diff
  • DKCR-fast-depth on sw-lin-mesa: diff
  • ed-updated on sw-lin-mesa: diff
  • fifa-street on sw-lin-mesa: diff
  • find-mii on sw-lin-mesa: diff
  • fishing-resort-map on sw-lin-mesa: diff
  • fortune-street on sw-lin-mesa: diff
  • fortune-street-fog on sw-lin-mesa: diff
  • fortune-street-white-box on sw-lin-mesa: diff
  • fsa-layers on sw-lin-mesa: diff
  • f-zero-rain on sw-lin-mesa: diff
  • inverted-depth-range on sw-lin-mesa: diff
  • jd2-fmv on sw-lin-mesa: diff
  • jj-awae-mirrored on sw-lin-mesa: diff
  • kirby-shadows on sw-lin-mesa: diff
  • last-story-shadows on sw-lin-mesa: diff
  • lego-star-wars-crane-shadow on sw-lin-mesa: diff
  • lesson08 on sw-lin-mesa: diff
  • lm-mario-portrait on sw-lin-mesa: diff
  • luigi-shadows on sw-lin-mesa: diff
  • mario-baseball-shadows on sw-lin-mesa: diff
  • mario-sluggers-bar on sw-lin-mesa: diff
  • mario-tennis-menu on sw-lin-mesa: diff
  • MaS-LOG-wiimote on sw-lin-mesa: diff
  • medabots-crash on sw-lin-mesa: diff
  • megaman-heat on sw-lin-mesa: diff
  • melee-depth on sw-lin-mesa: diff
  • melee-lighting on sw-lin-mesa: diff
  • metroid-visor on sw-lin-mesa: diff
  • mii-channel on sw-lin-mesa: diff
  • milotic-texture on sw-lin-mesa: diff
  • mini-ninjas on sw-lin-mesa: diff
  • mkdd-babypark on sw-lin-mesa: diff
  • mkdd-efb on sw-lin-mesa: diff
  • mkw-bridge on sw-lin-mesa: diff
  • mkwii-bluebox on sw-lin-mesa: diff
  • monkeyball-fuse on sw-lin-mesa: diff
  • mp3-bloom on sw-lin-mesa: diff
  • mp7-text on sw-lin-mesa: diff
  • mtennis-zfreeze on sw-lin-mesa: diff
  • nddemo-bumpmapping on sw-lin-mesa: diff
  • nddemo-lighting on sw-lin-mesa: diff
  • nfsu-purplerect on sw-lin-mesa: diff
  • nfsu-reflections on sw-lin-mesa: diff
  • nhl-slap on sw-lin-mesa: diff
  • nsmbw-coins on sw-lin-mesa: diff
  • nsmbw-intro on sw-lin-mesa: diff
  • pm-hc-jp on sw-lin-mesa: diff
  • rs2-bumpmapping on sw-lin-mesa: diff
  • rs2-glass on sw-lin-mesa: diff
  • rs2-skybox on sw-lin-mesa: diff
  • rs2-zfreeze on sw-lin-mesa: diff
  • rs3-bumpmapping on sw-lin-mesa: diff
  • rs3-skybox2 on sw-lin-mesa: diff
  • sadx-ui on sw-lin-mesa: diff
  • sfa-shadows on sw-lin-mesa: diff
  • sf-assault-flashing on sw-lin-mesa: diff
  • shadow-eyes on sw-lin-mesa: diff
  • simpsons-game on sw-lin-mesa: diff
  • smg2-fog on sw-lin-mesa: diff
  • smg-marioeyes on sw-lin-mesa: diff
  • sms-bubbles on sw-lin-mesa: diff
  • sms-gc on sw-lin-mesa: diff
  • sms-water on sw-lin-mesa: diff
  • soa-black on sw-lin-mesa: diff
  • soniccolors-mm on sw-lin-mesa: diff
  • sonic-riders-blur on sw-lin-mesa: diff
  • sonicriderszg-gb on sw-lin-mesa: diff
  • spyro-bloom on sw-lin-mesa: diff
  • spyro-depth on sw-lin-mesa: diff
  • ssbb-mod-lloyd on sw-lin-mesa: diff
  • ssbm-pointsize on sw-lin-mesa: diff
  • ss-map on sw-lin-mesa: diff
  • ss-timestone on sw-lin-mesa: diff
  • super-sluggers-white-out on sw-lin-mesa: diff
  • sw3-dt on sw-lin-mesa: diff
  • thps3-earlyz on sw-lin-mesa: diff
  • thps4-shadow on sw-lin-mesa: diff
  • tla-menu on sw-lin-mesa: diff
  • tos-invis-char on sw-lin-mesa: diff
  • tp-skin on sw-lin-mesa: diff
  • vegas-party-depth on sw-lin-mesa: diff
  • viewitful-joe-distortion on sw-lin-mesa: diff
  • xenoblade-menu on sw-lin-mesa: diff
  • ztp-grass on sw-lin-mesa: diff
  • zww-armos on sw-lin-mesa: diff
  • zww-water on sw-lin-mesa: diff
  • zww-waves on sw-lin-mesa: diff

automated-fifoci-reporter

@Tilka Tilka merged commit c42b1c1 into dolphin-emu:master Jul 25, 2021
11 checks passed
@phire
Copy link
Member

phire commented Jul 25, 2021

It's important to note that Mirrored with height=96 produces a quite different result on hardware.

@Pokechu22
Copy link
Contributor Author

Here are some additional images from the hardware fifoplayer: WrongTitle_191_192.7z.zip WrongTitle_96.7z.zip

With height=96, when repeat is used there does seem to be some randomness (though it still matches the pattern behind it, as I found). With mirror, it seems to be consistent (though the different pattern from what Dolphin shows still exists).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants