Add configurable toggle to round vertices to nearest pixel #4715

iwubcode · 2017-01-22T22:50:19Z

This is the result of investigating why there's corruption in some games at higher IR (Spongebob, SilentHill) which often occurs during an efb copy. What I ended up finding out was that our pixel offset was not properly taking into account higher IRs correctly. At 6x IR for instance, we were often off by 3 pixels.

I believe this occurs because the Wii vertices often have higher precision than what the Wii resolution could show. In order to combat this, we scale the vertices to the native resolution before they get handed off to the pixel shader and outputted in the higher resolution.

Since this fix only helps a handful of games specifically at higher IR, we've decided to make this a toggle.

iwubcode · 2017-01-23T00:34:13Z

Fixes issues: https://bugs.dolphin-emu.org/issues/9357 , https://bugs.dolphin-emu.org/issues/9649 , https://bugs.dolphin-emu.org/issues/8710

MayImilae · 2017-01-23T00:35:57Z

Can you provide some images of what this effects?

Source/Core/VideoCommon/BPStructs.cpp

-    // resolution.
-    // TODO: What's the behavior of out of bound access?
+    // The width/height of the region is off by 1 from what the game developer actually intended
+    // so add 1 to compensate


iwubcode · 2017-01-23T01:01:38Z

@MayImilae - here's some comparisons of what I've found...I'm planning on doing more testing of other areas/games:

(see below for updated comparisons)

ghost · 2017-01-23T21:20:46Z

Is it possible to use the FIFO CI system for higher IR screens? This seems like something that would be a good candidate for that kind of testing.

JMC47 · 2017-01-25T21:52:21Z

You'd have to ask @delroth about the viability of a special bot for that.

Lee91x · 2017-02-08T03:09:48Z

Does this fix the pixel border in TS2 ?
http://imgur.com/kRZOJo7

iwubcode · 2017-02-08T03:31:16Z

@Lee91x - which game is that from?

Feel free to test it and let me know!

MayImilae · 2017-02-08T06:11:00Z

I do believe TS2 stands for "TimeSplitters 2"

https://en.wikipedia.org/wiki/TimeSplitters_2
https://wiki.dolphin-emu.org/index.php?title=TimeSplitters_2

Source/Core/VideoCommon/VertexShaderGen.cpp

+  // however, higher resolutions than the Wii outputs
+  // cause an additional pixel offset
+  // due to a higher pixel density
+  // we need to correc this by converting our


Source/Core/VideoCommon/VertexShaderGen.cpp

+  // clip-space position into the Wii's screen-space
+  // acquire the right pixel and then convert it back
+  out.Write("float ss_pixel_x = ((o.pos.x + 1.0f) * (640.0f * 0.5f));\n");
+  out.Write("float ss_pixel_y = ((o.pos.y + 1.0f) * (480.0f * 0.5f));\n");


dolphin-emu-bot · 2017-02-19T03:27:31Z

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

ab11-homebrew on ogl-lin-mesa: diff
aeon-charge-attack on ogl-lin-mesa: diff
burnout2-vehicletextures on ogl-lin-mesa: diff
chibi-robo-fastdepth on ogl-lin-mesa: diff
chibi-robo-zfighting on ogl-lin-mesa: diff
custom-brawl-char on ogl-lin-mesa: diff
dbz-depth on ogl-lin-mesa: diff
djfny-menu on ogl-lin-mesa: diff
DKCR-Char on ogl-lin-mesa: diff
DKCR-fast-depth on ogl-lin-mesa: diff
ed-updated on ogl-lin-mesa: diff
et-vid on ogl-lin-mesa: diff
fifa-street on ogl-lin-mesa: diff
find-mii on ogl-lin-mesa: diff
fishing-resort-map on ogl-lin-mesa: diff
fog-adj on ogl-lin-mesa: diff
fortune-street on ogl-lin-mesa: diff
fortune-street-fog on ogl-lin-mesa: diff
fortune-street-white-box on ogl-lin-mesa: diff
fsa-layers on ogl-lin-mesa: diff
f-zero-rain on ogl-lin-mesa: diff
goldeneye-depth on ogl-lin-mesa: diff
hb-discgolf on ogl-lin-mesa: diff
inverted-depth-range on ogl-lin-mesa: diff
jj-awae-mirrored on ogl-lin-mesa: diff
kirby-shadows on ogl-lin-mesa: diff
last-story-shadows on ogl-lin-mesa: diff
lego-star-wars-crane-shadow on ogl-lin-mesa: diff
lesson08 on ogl-lin-mesa: diff
line-width-test on ogl-lin-mesa: diff
luigi-shadows on ogl-lin-mesa: diff
mario-baseball-shadows on ogl-lin-mesa: diff
mario-sluggers-bar on ogl-lin-mesa: diff
mario-tennis-menu on ogl-lin-mesa: diff
MaS-LOG-wiimote on ogl-lin-mesa: diff
medabots-crash on ogl-lin-mesa: diff
megaman-heat on ogl-lin-mesa: diff
melee-depth on ogl-lin-mesa: diff
melee-lighting on ogl-lin-mesa: diff
metroid-visor on ogl-lin-mesa: diff
mii-channel on ogl-lin-mesa: diff
milotic-texture on ogl-lin-mesa: diff
mini-ninjas on ogl-lin-mesa: diff
mkdd-babypark on ogl-lin-mesa: diff
mkdd-efb on ogl-lin-mesa: diff
mkwii-bluebox on ogl-lin-mesa: diff
monkeyball-fuse on ogl-lin-mesa: diff
mp3-bloom on ogl-lin-mesa: diff
mp7-text on ogl-lin-mesa: diff
mtennis-zfreeze on ogl-lin-mesa: diff
my-word-coach on ogl-lin-mesa: diff
nddemo-bumpmapping on ogl-lin-mesa: diff
nddemo-lighting on ogl-lin-mesa: diff
nes-vc on ogl-lin-mesa: diff
nfsu-purplerect on ogl-lin-mesa: diff
nfsu-reflections on ogl-lin-mesa: diff
nsmbw-coins on ogl-lin-mesa: diff
nsmbw-intro on ogl-lin-mesa: diff
pm-hc-jp on ogl-lin-mesa: diff
pw-black-bars on ogl-lin-mesa: diff
rs2-bumpmapping on ogl-lin-mesa: diff
rs2-glass on ogl-lin-mesa: diff
rs2-skybox on ogl-lin-mesa: diff
rs2-zfreeze on ogl-lin-mesa: diff
rs3-bumpmapping on ogl-lin-mesa: diff
rs3-skybox2 on ogl-lin-mesa: diff
sadx-ui on ogl-lin-mesa: diff
sfa-shadows on ogl-lin-mesa: diff
sf-assault-flashing on ogl-lin-mesa: diff
simpsons-game on ogl-lin-mesa: diff
smg2-fog on ogl-lin-mesa: diff
smg-marioeyes on ogl-lin-mesa: diff
sms-bubbles on ogl-lin-mesa: diff
sms-gc on ogl-lin-mesa: diff
sms-water on ogl-lin-mesa: diff
soa-black on ogl-lin-mesa: diff
soniccolors-mm on ogl-lin-mesa: diff
sonic-riders-blur on ogl-lin-mesa: diff
sonicriderszg-gb on ogl-lin-mesa: diff
spyro-bloom on ogl-lin-mesa: diff
spyro-depth on ogl-lin-mesa: diff
ssbb-mod-lloyd on ogl-lin-mesa: diff
ssbm-pointsize on ogl-lin-mesa: diff
ss-map on ogl-lin-mesa: diff
ss-timestone on ogl-lin-mesa: diff
super-sluggers-white-out on ogl-lin-mesa: diff
sw3-dt on ogl-lin-mesa: diff
taiko-depth on ogl-lin-mesa: diff
thps3-earlyz on ogl-lin-mesa: diff
thps4-shadow on ogl-lin-mesa: diff
tla-menu on ogl-lin-mesa: diff
tos-invis-char on ogl-lin-mesa: diff
tsp3-pinkgrass on ogl-lin-mesa: diff
vegas-party-depth on ogl-lin-mesa: diff
xenoblade-menu on ogl-lin-mesa: diff
zelda1-vc on ogl-lin-mesa: diff
ztp-grass on ogl-lin-mesa: diff
zww-armos on ogl-lin-mesa: diff
zww-water on ogl-lin-mesa: diff
zww-waves on ogl-lin-mesa: diff

_{^{automated-fifoci-reporter}}

iwubcode · 2017-02-19T03:57:36Z

@phire and I were talking about this and he informed me that my math for converting to screenspace from clip-space was wrong because I was not including 'w' into the calculation. This wasn't intentional but after messing around with it, adding 'w' gives jumpy effects to some vertices. So I plan on leaving it out.

Phire commented on what effect this would have:

basically, if you ignore w, then elements with w=1 will get your rounding applied
and elements with w=2 will get half your rounding applied
and elements with w=3 will get 1/3rd of your rounding applied
and so on

Lastly, this was never meant for 1x resolution. I left it in, to get an idea of what kind of impact my changes would have. Ideally, there'd be no changes in the fifolog but unfortunately that seems to be very difficult to achieve. How would other Dolphin developers feel about adding this as a hack to fix these additional games at higher IR? (as in, only generate this shader code when higher irs are chosen)

@phire did mention we could potentially do this with a "dynamic offset", I need to talk to him more about how he would decide what offset-to-apply on which object.

iwubcode · 2017-02-19T05:49:29Z

@JMC47 - tested this and says this fixes "Mario Kart Wii" menus at 3x+, "Sonic Heroes" and "Shadow the Hedgehog".

iwubcode · 2017-02-25T22:53:41Z

Updated comparisons:

Mario Kart Wii

Example 1

Master 1x (directx)

http://i.imgur.com/VVbHVYB.png

Master 3x (directx)

http://i.imgur.com/Lo5uQQQ.png

This PR 3x (directx)

http://i.imgur.com/50j1Jgu.png

Example 2

Master 1x (directx)

http://i.imgur.com/cl4QQNM.png

Master 3x (directx)

http://i.imgur.com/W3h5IOZ.png

This PR 3x (directx)

http://i.imgur.com/c03b6a4.png

Silent Hill Shattered Memories

Example 1

Master 1x (directx)

http://i.imgur.com/VaMirGD.png

Master 3x (directx)

http://i.imgur.com/pbeZoTb.png

This PR 3x (directx)

http://i.imgur.com/C88pxAe.png

Example 2

Master 1x (directx)

http://i.imgur.com/RrRbRhs.png

Master 3x (directx)

http://i.imgur.com/O8CYhFY.png

This PR 3x (directx)

http://i.imgur.com/zrig3B8.png

Spongebob

Master 1x (directx)

http://i.imgur.com/tMQDHwf.png

Master 3x (directx)

http://i.imgur.com/l0jQpnf.png

This PR 3x (directx)

http://i.imgur.com/Iylk4Jw.png

No known major regressions

JMC47 · 2017-04-04T00:35:51Z

@degasus @Armada651 @stenzek are we okay with something like this?

Source/Core/DolphinWX/VideoConfigDiag.cpp

@@ -284,6 +284,9 @@ static wxString true_color_desc =
    wxTRANSLATE("Forces the game to render the RGB color channels in 24-bit, thereby increasing "
                "quality by reducing color banding.\nIt has no impact on performance and causes "
                "few graphical issues.\n\n\nIf unsure, leave this checked.");
+static wxString vertex_rounding_desc =
+    wxTRANSLATE("Round vertices to whole pixels.  Fixes some "


Source/Core/VideoCommon/VertexShaderGen.cpp

@@ -465,6 +465,29 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const vertex_shader_uid_da
  // get rasterized correctly.
  out.Write("o.pos.xy = o.pos.xy - o.pos.w * " I_PIXELCENTERCORRECTION ".xy;\n");

+  if (g_ActiveConfig.bVertexRounding)


Source/Core/VideoCommon/VertexShaderGen.h

@@ -38,6 +38,7 @@ struct vertex_shader_uid_data
  u32 numColorChans : 2;
  u32 dualTexTrans_enabled : 1;
  u32 pixel_lighting : 1;
+  u32 vertex_rounding : 1;


Source/Core/VideoCommon/VertexShaderGen.cpp

@@ -465,6 +466,29 @@ ShaderCode GenerateVertexShaderCode(APIType api_type, const vertex_shader_uid_da
  // get rasterized correctly.
  out.Write("o.pos.xy = o.pos.xy - o.pos.w * " I_PIXELCENTERCORRECTION ".xy;\n");

+  if (uid_data->vertex_rounding && g_ActiveConfig.iEFBScale != SCALE_1X)


Source/Core/VideoCommon/VertexShaderGen.cpp

@@ -28,6 +28,7 @@ VertexShaderUid GetVertexShaderUid()
  uid_data->numTexGens = xfmem.numTexGen.numTexGens;
  uid_data->components = VertexLoaderManager::g_current_components;
  uid_data->pixel_lighting = g_ActiveConfig.bEnablePixelLighting;
+  uid_data->vertex_rounding = g_ActiveConfig.bVertexRounding;


phire · 2017-04-04T04:11:07Z

Actually, can we change how the GUI for this works.

It need to handle the following case:

User sets IR to 2x
User checks Round vertices
Playing the game results in rounded vertices
User changes IR to 1x
Round vertices feature is automatically disabled.
playing the game works as expected
User changes the IR back to 2x
Round vertices feature is enabled again without the user having to check the box.

Either we leave the UI alone and the checkbox is always clickable -- Or -- we gray it out whenever the IR is set to 1x (checkbox says checked or unchecked, whatever it previously was)

Source/Core/VideoCommon/ConstantManager.h

@@ -45,6 +46,7 @@ struct VertexShaderConstants
  float4 normalmatrices[32];
  float4 posttransformmatrices[64];
  float4 pixelcentercorrection;
+  float2 viewport;


Source/Core/VideoCommon/ConstantManager.h

@@ -8,6 +8,7 @@

 // all constant buffer attributes must be 16 bytes aligned, so this are the only allowed components:
 typedef float float4[4];
+typedef float float2[2];


Source/Core/VideoCommon/VertexShaderManager.cpp

@@ -382,15 +382,24 @@ void VertexShaderManager::SetConstants()
    // NOTE: If we ever emulate antialiasing, the sample locations set by
    // BP registers 0x01-0x04 need to be considered here.
    const float pixel_center_correction = 7.0f / 12.0f - 0.5f;
-    const float pixel_size_x = 2.f / g_renderer->EFBToScaledXf(2.f * xfmem.viewport.wd);
-    const float pixel_size_y = 2.f / g_renderer->EFBToScaledXf(2.f * xfmem.viewport.ht);
+    const float viewport_width = g_ActiveConfig.bVertexRounding ?


Source/Core/VideoCommon/VertexShaderManager.cpp

+    const float viewport_width = g_ActiveConfig.bVertexRounding ?
+                                     (2.f * xfmem.viewport.wd) :
+                                     g_renderer->EFBToScaledXf(2.f * xfmem.viewport.wd);
+    const float viewport_height = g_ActiveConfig.bVertexRounding ?


Source/Core/DolphinWX/VideoConfigDiag.cpp

@@ -284,6 +284,9 @@ static wxString true_color_desc =
    wxTRANSLATE("Forces the game to render the RGB color channels in 24-bit, thereby increasing "
                "quality by reducing color banding.\nIt has no impact on performance and causes "
                "few graphical issues.\n\n\nIf unsure, leave this checked.");
+static wxString vertex_rounding_desc =
+    wxTRANSLATE("Round 2D vertices to whole pixels.  Fixes some "
+                "games at higher internal resolutions.\n\nIf unsure, leave this unchecked.");


iwubcode · 2017-04-04T05:25:34Z

All of @phire 's requested changes are in.

Also went with phire's suggestion of "we gray it out whenever the IR is set to 1x" and keep the checkbox at whatever it was previously set to.

phire

All your fixes look good, but I managed to find a few extra issues.

Source/Core/VideoCommon/VertexShaderGen.cpp

+    out.Write("if (o.pos.w == 1.0f)\n");
+    out.Write("{\n");
+
+    out.Write("float ss_pixel_x = ((o.pos.x + 1.0f) * (" I_VIEWPORT_SIZE ".x * 0.5f));\n");


Source/Core/DolphinWX/VideoConfigDiag.h

+    else
+    {
+      vertex_rounding_checkbox->Enable(true);
+      vconfig.bVertexRounding = vertex_rounding_checkbox->GetValue();


w=1. This fixes some games at higher IRs.

iwubcode · 2017-04-04T15:01:08Z

In addition to @phire 's changes, I also moved the Vertex Rounding option to be under hacks/other (requested by @JMC47 )

phire · 2017-04-05T03:00:01Z

Code looks good to me, but I haven't tested it.

If everything is working as expected, it can be merged.

JMC47 · 2017-04-05T07:41:17Z

LGTM. Performs pretty much as expected, turns off at 1x IR.

escape209 · 2017-04-12T21:47:07Z

Before Fix:

After Fix:

Oddly enough, in Burnout 2, this fixes an issue in the main menu (notice the pixel-wide gap on the left where the background is showing through. It's not very noticeable in this image, but it is when in motion), but introduces another issue (the widths of the letters are off, take a look at the letters in "Acceleration.")

iwubcode · 2017-04-13T02:16:17Z

@disorderIy - the change in UI is expected and is known to happen in most games. (All the games listed by the bot above were games that had UI changes by this hack)

I didn't realize Burnout2 was one of the games that benefited from the hack, thanks for letting me know.

phire · 2017-04-13T02:48:02Z

Slightly annoying that the font gets worse while the line gets better, but that's kind of expected. (Looks like Burnout 2 is doing sub-pixel rendering of the font, which this hack destroys)

Parlane added the WIP / do not merge Work in progress (do not merge) label Jan 22, 2017

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch from aaa428d to 92a79df Compare January 22, 2017 22:59

magcius reviewed Jan 23, 2017

View reviewed changes

Source/Core/VideoCommon/BPStructs.cpp Outdated

// resolution.

// TODO: What's the behavior of out of bound access?

// The width/height of the region is off by 1 from what the game developer actually intended

// so add 1 to compensate

This comment was marked as off-topic.

Sign in to view

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 3 times, most recently from bebc912 to 863414d Compare January 29, 2017 21:05

iwubcode changed the title ~~[Test do not merge] Fix pixel offset at higher IRs~~ [Test do not merge] Fix efb offset at higher IRs Feb 9, 2017

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch from 863414d to 19db701 Compare February 9, 2017 04:39

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 2 times, most recently from 01b44dd to 6785644 Compare February 19, 2017 00:26

iwubcode changed the title ~~[Test do not merge] Fix efb offset at higher IRs~~ Fix efb offset at higher IRs Feb 19, 2017

ghost reviewed Feb 19, 2017

View reviewed changes

phire reviewed Feb 19, 2017

View reviewed changes

iwubcode changed the title ~~Fix efb offset at higher IRs~~ Fix offset at higher IRs Feb 19, 2017

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 2 times, most recently from de8e2d6 to c667906 Compare February 26, 2017 01:07

iwubcode changed the title ~~Fix offset at higher IRs~~ Round vertices to nearest pixel to fix some games at higher IRs Feb 26, 2017

iwubcode changed the title ~~Round vertices to nearest pixel to fix some games at higher IRs~~ Add configurable toggle to round vertices to nearest pixel Feb 26, 2017

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch from e28f1ec to bd94366 Compare April 4, 2017 00:31

phire reviewed Apr 4, 2017

View reviewed changes

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 2 times, most recently from 092f916 to e3c6006 Compare April 4, 2017 03:51

phire requested changes Apr 4, 2017

View reviewed changes

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 2 times, most recently from df608a9 to b92e20a Compare April 4, 2017 04:28

phire requested changes Apr 4, 2017

View reviewed changes

phire reviewed Apr 4, 2017

View reviewed changes

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch 4 times, most recently from 2fecc6f to 20e6f24 Compare April 4, 2017 05:20

phire reviewed Apr 4, 2017

View reviewed changes

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch from 20e6f24 to fa6c38a Compare April 4, 2017 14:48

Add configurable toggle that rounds vertices to the nearest pixel when

a9d08a3

w=1. This fixes some games at higher IRs.

iwubcode force-pushed the efb_copy_corruption_at_higher_ir branch from fa6c38a to a9d08a3 Compare April 4, 2017 14:52

phire approved these changes Apr 5, 2017

View reviewed changes

phire merged commit 4c0a392 into dolphin-emu:master Apr 5, 2017

iwubcode deleted the efb_copy_corruption_at_higher_ir branch April 6, 2017 01:42

Pokechu22 mentioned this pull request Nov 21, 2020

FifoCI no longer leaves comments on PRs where changes occur; /json endpoint 500s when there are changes dolphin-emu/fifoci#30

Closed

Add configurable toggle to round vertices to nearest pixel #4715

Add configurable toggle to round vertices to nearest pixel #4715

Conversation

iwubcode commented Jan 22, 2017 • edited Loading

iwubcode commented Jan 23, 2017 • edited Loading

MayImilae commented Jan 23, 2017

This comment was marked as off-topic.

iwubcode commented Jan 23, 2017 • edited Loading

ghost commented Jan 23, 2017

JMC47 commented Jan 25, 2017

Lee91x commented Feb 8, 2017

iwubcode commented Feb 8, 2017

MayImilae commented Feb 8, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

dolphin-emu-bot commented Feb 19, 2017

iwubcode commented Feb 19, 2017 • edited Loading

iwubcode commented Feb 19, 2017 • edited Loading

iwubcode commented Feb 25, 2017 • edited Loading

JMC47 commented Apr 4, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

phire commented Apr 4, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

iwubcode commented Apr 4, 2017

phire left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

iwubcode commented Apr 4, 2017

phire commented Apr 5, 2017

JMC47 commented Apr 5, 2017

escape209 commented Apr 12, 2017 • edited Loading

iwubcode commented Apr 13, 2017 • edited Loading

phire commented Apr 13, 2017

iwubcode commented Jan 22, 2017 •

edited

Loading

iwubcode commented Jan 23, 2017 •

edited

Loading

iwubcode commented Jan 23, 2017 •

edited

Loading

iwubcode commented Feb 19, 2017 •

edited

Loading

iwubcode commented Feb 19, 2017 •

edited

Loading

iwubcode commented Feb 25, 2017 •

edited

Loading

escape209 commented Apr 12, 2017 •

edited

Loading

iwubcode commented Apr 13, 2017 •

edited

Loading