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

Vertices are all over the place #91

Open
AloXado320 opened this issue Dec 11, 2024 · 14 comments
Open

Vertices are all over the place #91

AloXado320 opened this issue Dec 11, 2024 · 14 comments

Comments

@AloXado320
Copy link

AloXado320 commented Dec 11, 2024

Bug Report

What's the issue you encountered?

I tried to update an unfinished port of GC/Wii of SM64 due to lack of GX knowledge using a legacy OpenGL render and SDL2. Though there's a warning in Dolphin, the game seems to render with some problems, like half of screen is filled with a black box and most notably, there are a lot of vertex explosions and it's more noticeable when the camera is upclose.

How can the issue be reproduced?

Compile the Wii port on my repo https://github.com/AloXado320/sm64-port/tree/gc_wii
make -j4 TARGET_WII=1 ENABLE_OPENGX=1
Instructions should be the same as the sm64 decomp project.

Environment?

I'm using Windows 11 using WSL2 Ubuntu, I compiled latest opengx since the one on devkitpro's package is outdated.

Additional context?

[N/A]

@mardy
Copy link
Collaborator

mardy commented Dec 11, 2024

Hi Alo! That's an interesting project! It will take me a few days before I can look into this, would you mind sharing a video exposing the problem?

Also, the OpenGL module that you added in your branch, does it work fine on the desktop? (just asking if there could be errors in it)

@AloXado320
Copy link
Author

Hi Alo! That's an interesting project! It will take me a few days before I can look into this, would you mind sharing a video exposing the problem?

Sure, here's the game running on Dolphin

2024-12-09.20-37-28-00.00.01.145-00.01.14.652.mp4

Also, the OpenGL module that you added in your branch, does it work fine on the desktop? (just asking if there could be errors in it)

Yeah, it was taken from another port and I tested it before on my PC and using Mesa software render on a VM.

@mardy
Copy link
Collaborator

mardy commented Dec 12, 2024

Wow, that's amazing! Good job! I tested it locally, and it works better than in your video :-) The vertices are still all over the place, but the issue with the screen being half black is fixed. Did you try on a real wii? (I'll try myself soon!)

Note that opengx requires a couple of Dolphin settings in order to work properly (I should actually document them in the readme...):

  • Graphics -> Hacks -> Texture Cache -> Accuracy: move it all towards "Safe"
  • Graphics -> Hacks -> Embedded Frame Buffer (EFB) -> Uncheck "Store EFB copies to texture only"
  • Graphics -> Hacks -> External Frame Buffer (XFB) -> Uncheck "Store XFB copies to texture only"
  • Graphics -> Hacks -> Other -> Uncheck "Disable Bounding Box"

In the "Graphics -> Enhancements" page I've also per-pixel lighting enabled, though I'm not sure whether it's needed.

The vertex issue seems due to clipping, it looks like not all polygons are drawn, so you can see the things behind them. I suspect the near clipping plane is computed wrong, I'll try to play with it.

@mardy
Copy link
Collaborator

mardy commented Dec 12, 2024

On the Wii it doesn't work at all, due to errors happening early on (with Dolphin we can ignore them, but on a Wii they case a crash). Do you see them too?

I'm attaching a screenshot, where the call stack is also visible. Maybe SDL is not being properly initialized?
screen

@mardy
Copy link
Collaborator

mardy commented Dec 12, 2024

OK, to fix the initialization issue and get the game to run on a Wii, I did:

diff --git a/Makefile b/Makefile
index c115f5a..7cdafa0 100644
--- a/Makefile
+++ b/Makefile
@@ -509,7 +509,7 @@ ifeq ($(TARGET_GX),1)
   endif
   ifeq ($(ENABLE_OPENGX),1)
     PLATFORM_CFLAGS  := $(MACHDEP) -DTARGET_GX  -DENABLE_OPENGX -fomit-frame-pointer -fno-strict-aliasing -I$(LIBOGC)/include -I$(PORTLIBS)/include
-    PLATFORM_LDFLAGS += $(MACHDEP) -L$(PORTLIBS)/lib -lopengx -lSDL2 -L$(LIBOGC)/lib/$(GX_PLATFORM) -g -lm -lasnd -laesnd -lfat $(WII_LIBS) -logc
+    PLATFORM_LDFLAGS += $(MACHDEP) -L$(PORTLIBS)/lib -lopengx -lSDL2 -lSDL2main -L$(LIBOGC)/lib/$(GX_PLATFORM) -g -lm -lasnd -laesnd -lfat $(WII_LIBS) -logc
   else
     PLATFORM_CFLAGS  := $(MACHDEP) -DTARGET_GX -fomit-frame-pointer -fno-strict-aliasing -I$(LIBOGC)/include
     PLATFORM_LDFLAGS := $(MACHDEP) -L$(LIBOGC)/lib/$(GX_PLATFORM) -g -lm -lasnd $(WII_LIBS) -lfat -logc
diff --git a/src/pc/pc_main.c b/src/pc/pc_main.c
index b570fd1..79d3f26 100644
--- a/src/pc/pc_main.c
+++ b/src/pc/pc_main.c
@@ -22,6 +22,7 @@
 #include "gfx/gfx_gx_wm.h"
 #include "gfx/gfx_gx.h"
 #endif
+#include <SDL2/SDL.h>
 
 #include "audio/audio_api.h"
 #include "audio/audio_wasapi.h"

I don't think it's correct to include SDL.h from pc_main.c, it probably needs to be done from some other include file, but this works too :-)

@mardy
Copy link
Collaborator

mardy commented Dec 12, 2024

I have a branch with a proper implementation of glScissors, but it doesn't help with the exploding vertices. I also tried to forcefully enable and disable clipping, to play with the nar/far clipping planes, etc., but nothing helps.

I noticed that when the sandy floor gets visible, the texture on it appears to be stretched and as if its texture coordinates are changing rapidly. I suspect that it may be an issue of vertex coordinates being invalid, and that they get properly culled or clamped by a compliant OpenGL implementation, whereas opengx doesn't handle them correctly. But I'm still investigating (and soon running out of ideas). It might also be some rounding issue, since I see that these defects tend to appear on triangles being located quite near the camera field.

@mardy
Copy link
Collaborator

mardy commented Dec 14, 2024

Hi @AloXado320, I've build the Linux OpenGL legacy version and I'm comparing the vertices being sent for drawing. They are different, so it looks like there is some error somewhere else in the code.

I noticed that on the Wii version some vertices are being draw with a clipped value of z (that is, z / w) that is outside of the clipping range [-1, 1], so I added some debugging code to print them:

diff --git a/src/pc/gfx/gfx_opengl_legacy.c b/src/pc/gfx/gfx_opengl_legacy.c
index 92d398e..e458600 100644
--- a/src/pc/gfx/gfx_opengl_legacy.c
+++ b/src/pc/gfx/gfx_opengl_legacy.c
@@ -475,6 +475,16 @@ static void gfx_opengl_draw_triangles(float buf_vbo[], size_t buf_vbo_len, size_
     cur_buf_num_tris = buf_vbo_num_tris;
     cur_buf_stride = cur_buf_size / (3 * cur_buf_num_tris);
 
+    {
+        fprintf(stderr, "gfx_opengl_pass_mix_texture %d triangles\n", cur_buf_num_tris);
+        float *ptr = cur_buf;
+        for (int i = 0; i < cur_buf_num_tris * 3; i++) {
+            float z = ptr[2] / ptr[3];
+            if (z < -1.0 || z > 1.0)
+                fprintf(stderr, "  %f %f %f %f\n", ptr[0], ptr[1], ptr[2], ptr[3]);
+            ptr += cur_buf_stride / sizeof(float);
+        }
+    }
     gfx_opengl_apply_shader(cur_shader);
 
     // if there's two textures, set primary texture first

I applied the same patch both to the Linux and to the Wii version, and the results are different: on Linux, the if inside the for loop is never satisfied, whereas on the Wii we do end up there, exactly when the scene gets rendered incorrectly. For example, a small snippet from the output:

13:10:211 Core/HLE/HLE_OS.cpp:195 N[OSREPORT_HLE]: 800d3088->800d3070| gfx_opengl_pass_mix_texture 3 triangles                                                                                                                                                               
13:10:211 Core/HLE/HLE_OS.cpp:195 N[OSREPORT_HLE]: 800d3100->800d30e0|   4304.938477 977.626465 -2628.060059 -2402.905029                                                                                                                                                    
13:10:211 Core/HLE/HLE_OS.cpp:195 N[OSREPORT_HLE]: 800d3100->800d30e0|   4304.938477 977.626465 -2628.060059 -2402.905029                                                                                                                                                    
13:10:211 Core/HLE/HLE_OS.cpp:195 N[OSREPORT_HLE]: 800d3100->800d30e0|   -7334.251465 3371.702393 -7663.563477 -7388.303711                                                                                                                                                  
13:10:211 Core/HLE/HLE_OS.cpp:195 N[OSREPORT_HLE]: 800d3100->800d30e0|   4279.015625 969.147949 -2610.227539 -2385.250244                                                                                                                                                    

@mardy
Copy link
Collaborator

mardy commented Dec 14, 2024

My advice: try fixing your branch so that it can also be built on Linux, so that we can be 100% sure that the vertices passed to opengx are the same. Unfortunately, your tree is quite different from the DOS port you linked above, and I suspect that the bug is hidden in some of the changes. If you managed to build your branch for Linux, then it would be much easier to compare.

@AloXado320
Copy link
Author

My advice: try fixing your branch so that it can also be built on Linux, so that we can be 100% sure that the vertices passed to opengx are the same. Unfortunately, your tree is quite different from the DOS port you linked above, and I suspect that the bug is hidden in some of the changes. If you managed to build your branch for Linux, then it would be much easier to compare.

Will try on Windows, WSL2 and Dolphin.
Also the same guy who did the render also made the ps2 port alongside some gfx workgrounds, like manual clipping, it helps reducing the vertex explosions but they are still a problem so they will be under a define for now

@AloXado320
Copy link
Author

@mardy Linux should build now using make -j4 ENABLE_OPENGL_LEGACY=1

@mardy
Copy link
Collaborator

mardy commented Dec 15, 2024

Thanks @AloXado320, I can see that manual clipping helps a lot in restoring the missing polygons, and also removes some of the undesired polygons, but it's still not perfect (though, looking at the code, I cannot really find anything wrong with it): there are still cases left where z < -w or z > w. In most cases these are just rounding errors, because the two valus are nearly equal, but in other cases (where we still see errors in the graphics) I can see that z is well outside the clipping area, so there must be something wrong with that code.

I will also try to understand if this is a temporary workaround, or if it's required because of a limitation of the GX processor: I suspect that if all vertexes of a triangle are well outside of the viewport (but on opposite sides, meaning that we have a very big triangle that stretches all over the viewport), the GX clipping algorithm might not detect it and discard it completely; this would explain why the manual clipping helps. I'll do some tests on a simpler program to understand if this is the case.

A handful of other issues I noticed:

  • the code for glPolygonOffset in opengx is wrong, and it does not work properly when polygons are at a low (< 45 degrees) angle with the view; I actually knew this before, but it's not trivial to solve;
  • texture perspective correction does not work, and it's especially disturbing on large triangles. I'm afraid that it's again GX's fault, and that it activates perspective correction only when using a perspective projection matrix (sm64 uses an orthographic one). I'm afraid that to fix this you'll have to modify sm64's code to keep the projection matrix separate and apply it in hardware.
  • in your last commit, you accidentally broke support for the wii controller, it always uses the gamecube ones. But I think you could solve this problem by dropping these implementations and use the SDL controller. Incidentally, you should also try to use the SDL backend for audio, too; it should work.

@AloXado320
Copy link
Author

@mardy Wow, GX has some odd things going on it's scary, makes me wonder if other ports could potentially have the same matrix issue to workaround it.

I enabled gamecube controllers because I was thinking on how to use them together with wii controllers like in some games, I could use SDL2 controllers but I prefer to use native functions to use their exclusive features, I'll keep that in mind though.

I was also thinking of using SDL2 audio, though when I tried to use it there was some loud static noise, I could be missing something.

@mardy
Copy link
Collaborator

mardy commented Dec 16, 2024

@AloXado320 What I wrote above about clipping is just a theory of mine, and I'm not sure it's correct. I ran a quick test, modifying the triangle.c file in the wii-examples repository, and at a first sight GX behaves correctly. I need to look deeper into this.

I figured out where the error with software clipping is: it's not in the functions for manual clipping, but in the place where we set the clipping flags: the code does not take into account the fact that w might be negative. This fixes the issues for me:

diff --git a/src/pc/gfx/gfx_pc.c b/src/pc/gfx/gfx_pc.c
index d50ebb1..462d7df 100644
--- a/src/pc/gfx/gfx_pc.c
+++ b/src/pc/gfx/gfx_pc.c
@@ -42,7 +42,7 @@
 // Used on DOS and PS2 port, more or less a "temporary" workaround if enabled.
 #ifdef TARGET_GX
 // do manual clipping
-//#define GFX_MANUAL_CLIPPING 1
+#define GFX_MANUAL_CLIPPING 1
 #endif
 
 union RGBA {
@@ -691,8 +691,8 @@ static void gfx_sp_vertex(size_t n_vertices, size_t dest_index, const Vtx *verti
         if (x > w) d->clip_rej |= 2;
         if (y < -w) d->clip_rej |= 4;
         if (y > w) d->clip_rej |= 8;
-        if (z < -w) d->clip_rej |= 16;
-        if (z > w) d->clip_rej |= 32;
+        if (z/w < -1.0) d->clip_rej |= 16;
+        if (z/w > 1.0) d->clip_rej |= 32;
 
         d->x = x;
         d->y = y;

Indeed, all the other if's are also wrong, but so far I haven't noticed visual problems due to them.

As for the glPolygonOffset problem, I see that the OpenGL legacy driver has a check for the GL_ARB_multitexture extension, but then it doesn't use it. The best option is to actually use this extension if available (in opengx we do have it indeed), because then instead of drawing the same triangles a few times with a different Z offset, you'd draw them only once, and you'd get rid of any possible Z issues (which might still occur even if opengx properly implemented the glPolygonOffset function, when different objects are very close to each other in the Z direction). You can have a look at the OpenGL module in sm64, how it uses glActiveTexture() and calls glDrawArrays() just once.

@mardy
Copy link
Collaborator

mardy commented Dec 16, 2024

OK, I've now verified that if I set the othogonal projection matrix fields to be:

        proj[2][2] = 0.5;                                                                                                             
        proj[2][3] = -0.5;

then the clipping is all right and manual clipping is not needed. But I still have to figure out how/why this works. :-)

UPDATE: but indeed, these are also the values set by opengx. I got confused by my own changes when debugging the code. What is interesting here, is that indeed the software clipping is not needed, but what is needed is the fix from my previous comment about the negative w.

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

No branches or pull requests

2 participants