clean up

decompiler/IR2/AtomicOp.cpp

@@ -539,6 +539,8 @@ goos::Object AsmOp::to_form(const std::vector<DecompilerLabel>& labels, const En
     }
   }
 
+  // note: to correctly represent a MOVN/MOVZ in our IR, we need to both read and write the
+  // destination register, so we append a read to the end here.
   if (m_instr.kind == InstructionKind::MOVZ || m_instr.kind == InstructionKind::MOVN) {
     RegisterAccess ra(AccessMode::READ, m_dst->reg(), m_dst->idx());
     forms.push_back(ra.to_form(env));
@@ -610,6 +612,7 @@ void AsmOp::update_register_info() {
   }
 
   if (m_instr.kind == InstructionKind::MOVN || m_instr.kind == InstructionKind::MOVZ) {
+    // in the case that MOVN/MOVZ don't do the move, they effectively read the original value.
     m_read_regs.push_back(m_dst->reg());
   }
 
@@ -722,6 +725,8 @@ void AsmOp::collect_vars(RegAccessSet& vars) const {
   }
 
   if (m_instr.kind == InstructionKind::MOVN || m_instr.kind == InstructionKind::MOVZ) {
+    // the conditional moves read their write register, but don't have it listed as a write
+    // in the actual ASM.  We handle this difference for the variable naming system here.
     RegisterAccess ra(AccessMode::READ, m_dst->reg(), m_dst->idx());
     vars.insert(ra);
   }

docs/markdown/graphics.md

@@ -9,9 +9,9 @@ There are three frames in flight at a time:
 
 ## Synchronization
 
-The PC Port only synchronizes on vsync. This waits for the "draw" frame to finish rendering, and for the buffers to be swapped.
+The PC Port synchronizes on `syncv` and on `sync-path`. The `syncv` waits for an actual buffer swap and `sync-path` waits for the renderer to finish.
 
-The game's code for this is kind of messy and confusing. On the PS2, you make sure rendering is done with `sync-path`, which waits for the DMA chain to finish.  But they call this earlier than I think they need to, and I don't really understand why.  I don't see any place where they read back from the finished frame or depth buffer.  Or where they would overwrite the memory. There's a second call to `sync-path` right where you would expected, right before the `syncv`.  After `syncv`, they call some Sony library function to actually display the correct framebuffer, then immediately start sending the next DMA chain.
+The game's code for this is kind of messy and confusing, and calls `sync-path` twice. On the PS2, you make sure rendering is done with `sync-path`, which waits for the DMA chain to finish.  But they call this earlier than I think they need to, and I don't really understand why.  I don't see any place where they read back from the finished frame or depth buffer.  Or where they would overwrite the memory. There's a second call to `sync-path` right where you would expect, imeediately before the `syncv`.  After `syncv`, they call some Sony library function to actually display the correct framebuffer, then immediately start sending the next DMA chain.
 
 The stuff between `sync-path` and `syncv` is:
 - depth cue "calc" (seems fast)
@@ -30,13 +30,11 @@ The stuff between `sync-path` and `syncv` is:
 - cache flush
 - a second `sync-path`
 
-I'm really not sure why they have the first `sync-path` there. One theory is that they didn't want debug code and non-debug rendering running at the same time - the debug code will compete with the rendering to use the main bus, and will make the rendering slower.
-
-For now, the PC Port doesn't do anything on `sync-path`. I think there's two ways this could be an issue in the future:
-- If they reuse the DMA buffer after `sync-path`, and our renderer is still reading from it. Currently not an issue because of dma copy, described later.
-- If they need to read the completed depth buffer or frame buffer after `sync-path`. I don't see any example of this.
+According to the Performance Analyzer, this takes about 1% to 2% of a frame.  They subtract off 4% of a frame from the profile bar so that 100% there is really around 96% of a frame, I guess to account for this extra time.
 
+I'm really not sure why they have the first `sync-path` there.  It makes some sense in debug mode so that you can draw the profile bar for the GPU after it has finished.  Another theory is that they didn't want debug code and non-debug rendering running at the same time - the debug code will compete with the rendering to use the main bus, and will make the rendering slower.  But it seems like you don't want this in the release version.
 
+For now, the PC Port does sync on `sync-path`, but it probably doesn't need to.
 
 ## DMA Copy
 

game/graphics/dma/dma.h

@@ -2,7 +2,7 @@
 
 /*!
  * @file dma.h
- * Unpacking utils for PS2 DMA, VIF, and GIF data.
+ * PS2 DMA and VIF types.
  */
 
 #include <string>

game/graphics/dma/dma_chain_read.h

@@ -17,6 +17,9 @@
  * }
  */
 
+/*!
+ * Represents a DMA transfer, including 64-bits of VIF tag.
+ */
 struct DmaTransfer {
   const u8* data = nullptr;
   u32 data_offset = 0;

game/graphics/dma/dma_copy.cpp

@@ -3,18 +3,26 @@
 #include "dma_copy.h"
 #include "third-party/fmt/core.h"
 
+/*!
+ * Convert a DMA chain to an array of bytes that can be directly fed to VIF.
+ */
 std::vector<u8> flatten_dma(const DmaFollower& in) {
   DmaFollower state = in;
   std::vector<u8> result;
   while (!state.ended()) {
     auto read_result = state.read_and_advance();
-    result.push_back(0);  // tag transfer padding
-    result.push_back(read_result.transferred_tag);
-    result.insert(result.end(), read_result.data, read_result.data + read_result.size_bytes);
 
-    //    for (u32 i = 0; i < read_result.size_bytes; i += 8) {
-    //      result.push_back(state.read_val<u64>(read_result.data_offset + i));
-    //    }
+    // insert 1 quadword of zeros.
+    // the first 8 bytes will remain zero and will be interpreted as VIF nop.
+    for (int i = 0; i < 16; i++) {
+      result.push_back(0);
+    }
+
+    // the second will contain the transferred tag.
+    memcpy(result.data() + result.size() - 8, &read_result.transferred_tag, 8);
+
+    // and the actual DMA data.
+    result.insert(result.end(), read_result.data, read_result.data + read_result.size_bytes);
   }
   return result;
 }

game/graphics/dma/dma_copy.h

@@ -5,11 +5,6 @@
 #include "common/common_types.h"
 #include "game/graphics/dma/dma_chain_read.h"
 
-/*!
- * This function is used to copy a DMA chain. To avoid needing a huge amount of memory, the DMA data
- * is rearranged in memory.
- */
-
 struct DmaData {
   u32 start_offset = 0;
   std::vector<u8> data;
@@ -43,4 +38,7 @@ class FixedChunkDmaCopier {
   DmaData m_result;
 };
 
+/*!
+ * Convert a DMA chain to an array of bytes that can be directly fed to VIF.
+ */
 std::vector<u8> flatten_dma(const DmaFollower& in);

game/graphics/dma/gs.h

@@ -2,6 +2,11 @@
 
 #include "game/graphics/dma/dma.h"
 
+/*!
+ * @file gs.h
+ * PS2 GS/GIF hardware types
+ */
+
 struct GifTag {
   enum class Format : u8 { PACKED = 0, REGLIST = 1, IMAGE = 2, DISABLE = 3 };
 

game/graphics/types.h

@@ -1,6 +0,0 @@
-
-
-#ifndef JAK_TYPES_H
-#define JAK_TYPES_H
-
-#endif  // JAK_TYPES_H

goal_src/engine/game/main.gc

@@ -944,10 +944,6 @@
   )
 
 
-(defmacro dloop-dbg (str &rest args)
-  `(format 0 ,(string-append "[display-loop] " str) ,@args)
-  )
-
 (defun display-loop ()
   "This is in progress..."
 
@@ -961,9 +957,7 @@
     (set! *teleport* #t)
     (update-per-frame-settings! *setting-control*)
     ;;(init-time-of-day-context *time-of-day-context*) TODO
-    (format 0 "DISPLAY LOOP calling sync~%")
     (display-sync disp)
-    (format 0 "first call to sync done~%")
     (swap-display disp)
     ;; touching list
     ;; bler init