dolphin icon indicating copy to clipboard operation
dolphin copied to clipboard

VideoSoftware/Clipper: Convert into class

Open lioncash opened this issue 1 year ago • 5 comments

The clipper code is relatively self-contained and isn't super convoluted, so this can easily be turned into a class and housed within SetupUnit instances where it's primarily used.

lioncash avatar Dec 19 '23 21:12 lioncash

https://dolphin.ci/#/builders/35/builds/2305/steps/9/logs/stdio

Dolphin>>> 06:26:953 Core/Boot/Boot.cpp:652 N[BOOT]: Booting DFF: /var/lib/fifoci-worker/dff/SpiderMan2Viewport.dff
Dolphin>>> *** stack smashing detected ***: terminated
Dolphin>>> timeout: the monitored command dumped core
FIFO log playback failed for /var/lib/fifoci-worker/dff/SpiderMan2Viewport.dff
/var/lib/fifoci-worker/fifoci/fifoci/runner/linux/run_fifo_test.sh: line 75: 1232387 Aborted                 EGL_PLATFORM=surfaceless SEGFAULT_SIGNALS="abrt segv" $TIMEOUT_CMD $DOLPHIN -p headless -e $DFF &> >(show_logs Dolphin)

Pokechu22 avatar Dec 19 '23 23:12 Pokechu22

How would I get the dff in order to test the failure? It doesn't seem like the website allows for that (unless it's tucked away somewhere non-obvious)

lioncash avatar Dec 19 '23 23:12 lioncash

Seems like a genuine bug with the clipper creating too many vertices and breaking things. Not sure what (if anything) should be done about it, nor if it actually affects spiderman 2 or if there's just something wrong with that fifolog, given the garbage vertex. ... or maybe this is the cause of the garbage vertex?

Specifically to try to get better debug info, I did this:

diff --git a/Source/Core/VideoBackends/Software/Clipper.cpp b/Source/Core/VideoBackends/Software/Clipper.cpp
index c1ac58ab44..275807df4a 100644
--- a/Source/Core/VideoBackends/Software/Clipper.cpp
+++ b/Source/Core/VideoBackends/Software/Clipper.cpp
@@ -195,7 +195,7 @@ void Clipper::AddInterpolatedVertex(float t, int out, int in, int* num_vertices)
         continue;                                                                                  \
                                                                                                    \
       {                                                                                            \
-        int* tmp = inlist;                                                                         \
+        auto tmp = inlist;                                                                         \
         inlist = outlist;                                                                          \
         outlist = tmp;                                                                             \
         n = outcount;                                                                              \
@@ -242,8 +242,8 @@ void Clipper::ClipTriangle(int* indices, int* num_indices)
   {
     for (int i = 0; i < 3; i += 3)
     {
-      int vlist[2][2 * 6 + 1];
-      int *inlist = vlist[0], *outlist = vlist[1];
+      std::array <int, 2 * 6 + 1> vlist_in, vlist_out;
+      auto inlist = vlist_in.begin(), outlist = vlist_out.begin();
       int n = 3;
       int numVertices = 3;
 

and it fails in POLY_CLIP(CLIP_NEG_Z_BIT, 0, 0, 1, 1); with outlist still pointing to vlist_out.begin(), outcount = 14 (2*6+1 = 13, and since outcount is postincremented, this means that it wrote to index 13), n = 12, and numVertices = 15.

The clipping code is fairly confusing TBH; I tried to redo it over at #11221 but I don't remember what the state of that is currently.

How would I get the dff in order to test the failure? It doesn't seem like the website allows for that (unless it's tucked away somewhere non-obvious)

You have to go to https://fifo.ci/dff/ to find it, in this case https://fifo.ci/media/dff/SpiderMan2Viewport.dff. It's definitely not obvious (I'm not sure why there isn't a download button on https://fifo.ci/dff/inverted-depth-range/).

Pokechu22 avatar Dec 20 '23 00:12 Pokechu22

Hmm, poking at this, I'm not sure how to actually fix this (at least correctly). Though it is cool that converting it into a class uncovered an unrelated bug

lioncash avatar Dec 20 '23 01:12 lioncash

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

automated-fifoci-reporter

dolphin-ci[bot] avatar Dec 20 '23 01:12 dolphin-ci[bot]