Merge pull request #282 from undisbeliever/fix-inputs-new-nmi-isr

Input fixes and VBlank ISR rewrite

pvsneslib/include/snes/input.h

@@ -27,17 +27,20 @@
 
 ---------------------------------------------------------------------------------*/
 
-/*! \file input.h
-    \brief input support.
-*/
+/*!
+ * \file input.h
+ * \brief input support.
+ *
+ * The inputs are automatically read by the \ref VBlank-ISR on non-lag frames.
+ */
 
 #ifndef SNES_INPUT_INCLUDE
 #define SNES_INPUT_INCLUDE
 
 #include <snes/snestypes.h>
 #include <snes/interrupt.h>
 
-/*! \file
+/*!
     \brief common values for pad input.
 
     common values that can be used to test auto pad.
@@ -59,7 +62,7 @@ typedef enum KEYPAD_BITS
     KEY_Y = BIT(14),      //!< pad Y button.
 } KEYPAD_BITS;
 
-/*! \file
+/*!
     \brief common values for SuperScope input.
 */
 //! enum values for the SuperScope buttons and flags.
@@ -73,9 +76,9 @@ typedef enum SUPERSCOPE_BITS
     SSC_NOISE = BIT(8),     //!< superscope NOISE flag.
 } SUPERSCOPE_BITS;
 
-extern u16 pad_keys[2];
-extern u16 pad_keysold[2];
-extern u16 pad_keysrepeat[2];
+extern u16 pad_keys[5];     //!< current pad value
+extern u16 pad_keysold[5];  //!< previous pad value
+extern u16 pad_keysdown[5]; //!< newly pressed down pad keys
 
 extern u8 snes_mplay5;  /*! \brief 1 if MultiPlay5 is connected */
 extern u8 snes_mouse;   /*! \brief 1 if Mouse is going to be used */
@@ -147,59 +150,48 @@ extern u16 scope_sinceshot; /*! \brief Number of frames elapsed since last shot
 */
 #define REG_JOYxLH(a) (((vuint16 *)0x4218)[(a)])
 
-/*!	\fn scanPads()
-    \brief Wait for pad ready and read pad values in.
-*/
-void scanPads(void);
-
 /*!	\fn  padsCurrent(value)
     \brief Return current value of selected pad
-    \param value Address of the pad to use (0 or 1 to 4 if multiplayer 5 connected)
+    \param value pad index to use (0-1 or 0-4 if multiplayer 5 connected)
     \return unsigned short of the current pad value
 */
 // unsigned short padsCurrent(u16 value);
 #define padsCurrent(value) (pad_keys[value])
 
-/*!	\fn padsDown(u16 value)
+/*!	\fn padsDown(value)
     \brief Return value of down keys for selected pad
-    \param value Address of the pad to use (0 or 1 to 4 if multiplayer 5 connected)
-    \return unsigned short of the current pad value
+    \param value pad index to use (0-1 or 0-4 if multiplayer 5 connected)
+    \return unsigned short of the newly pressed down keys (0 -> 1 transition)
 */
-unsigned short padsDown(u16 value);
+// unsigned short padsDown(u16 value);
+#define padsDown(value) (pad_keysdown[value])
 
 /*!	\fn padsUp(u16 value)
     \brief Return value of up keys for selected pad
-    \param value Address of the pad to use (0 or 1 to 4 if multiplayer 5 connected)
-    \return unsigned short of the current pad value
+    \param value pad index to use (0-1 or 0-4 if multiplayer 5 connected)
+    \return unsigned short of the released keys (1 -> 0 transition)
 */
 unsigned short padsUp(u16 value);
 
 /*!	\fn padsClear(u16 value)
     \brief Clear internal variables for selected pad
-    \param value Address of the pad to use (0 or 1 to 4 if multiplayer 5 connected)
+    \param value pad index to use (0-1 or 0-4 if multiplayer 5 connected)
 */
 void padsClear(u16 value);
 
 /*!	\fn detectMPlay5(void)
     \brief Check if MultiPlayer5 is connected and populate snes_mplay5 (0 or 1 for connected)
-*/
-void detectMPlay5(void);
 
-/*!	\fn scanMPlay5()
-    \brief Wait for multiplayer5 pads ready and read pad values in.
+    \b CAUTION: REG_WRIO ($4201) must not be written to while MultiPlayer5 is active.
+    (Bit 7 of REG_WRIO must be set when Auto Joy reads the controllers, shortly after the VBlank Period starts.)
 */
-void scanMPlay5(void);
+void detectMPlay5(void);
 
 /*!	\fn detectMouse(void)
     \brief Check if Mouse is connected and populate snes_mouse (0 or 1 for connected)
 */
 void detectMouse(void);
 
-/*!	\fn mouseRead(void)
-    \brief Wait for mouse ready and read mouse values in.
-*/
-void mouseRead(void);
-
 /*!	\fn mouseSpeedChange(u8 port)
     \brief Set mouse hardware speed (populate mouseSpeed[] first).
     \param port Specify wich port to use (0-1)
@@ -211,33 +203,4 @@ void mouseSpeedChange(u8 port);
 */
 void detectSuperScope(void);
 
-/*!	\fn scanScope(void)
-    \brief  Nintendo SHVC Scope BIOS version 1.00
-     Quickly disassembled and commented by Revenant on 31 Jan 2013
-
-     This assembly uses xkas v14 syntax. It probably also assembles with bass, if there's
-     any such thing as good fortune in the universe.
-
-     How to use the SHVC Super Scope BIOS:
-     (all variables are two bytes)
-
-     1: Set "HoldDelay" and "RepDelay" for the button hold delay and repeat rate
-
-     2:  "jsr GetScope" or "jsl GetScopeLong" once per frame
-
-     3:  Read one of the following to get the scope input bits (see definitions below):
-         - ScopeDown (for any flags that are currently true)
-         - ScopeNow (for any flags that have become true this frame)
-         - ScopeHeld (for any flags that have been true for a certain length of time)
-         - ScopeLast (for any flags that were true on the previous frame)
-
-     3a: If the bits read from ScopeNow indicate a valid shot, or if the Cursor button
-         is being pressed, then read "ShotH"/"ShotV" to adjust for aim, or read
-         "ShotHRaw"/"ShotVRaw" for "pure" coordinates
- 
-     3c: at some point, set "CenterH"/"CenterV" equal to "ShotHRaw"/"ShotVRaw"
-         so that the aim-adjusted coordinates are "correct"
-*/
-void scanScope(void);
-
-#endif // SNES_PADS_INCLUDE
+#endif // SNES_PADS_INCLUDE

pvsneslib/include/snes/interrupt.h

@@ -27,15 +27,89 @@
 
 ---------------------------------------------------------------------------------*/
 
-/*! \file interrupt.h
-    \brief snes interrupt support.
-*/
+/*!
+ * \file interrupt.h
+ * \brief snes interrupt support.
+ *
+ * <h2>VBlank ISR \anchor VBlank-ISR</h2>
+ *
+ * PVSnesLib includes a Vertical Blank Interrupt Service Routine (VBlank ISR or NMI ISR) that will
+ * do the following actions (in this order) at the start of the Vertical Blanking Period when
+ * VBlank interrupts are enabled:
+ *  - Transfer #oamMemory to the PPU OAM on non lag-frames.
+ *  - Call #nmi_handler.
+ *  - Increment #snes_vblank_count.
+ *  - Read inputs from the controller ports on non lag-frames (see input.h).
+ *  - Increment #lag_frame_counter on lag-frames.
+ *
+ * A lag-frame is when the execution time between two WaitForVBlank() calls exceeds a 50/60Hz
+ * display frame.  By testing for lag-frames, the VBlank ISR will not transfer a partially
+ * populated #oamMemory to the PPU OAM.
+ *
+ * Lag-frames are determined by the #vblank_flag variable, which is set on WaitForVBlank() and
+ * cleared in the VBlank ISR.
+ *
+ * Inputs are only read on non lag-frames to prevent the input state from unexpectedly changing in
+ * the middle of the main loop (potentially causing a heisenbug).  Code that loops until a button
+ * is pressed must call WaitForVBlank() within the loop otherwise it will loop forever.
+ *
+ * The #nmi_handler function pointer (set using nmiSet()) is called on \b every VBlank interrupt
+ * (even during force-blank / setScreenOff()).
+ * To prevent glitches, #nmi_handler's should either:
+ *  - Test #vblank_flag and only update the PPU registers/memory if the #vblank_flag is set.
+ *  - Use locks or flags on every buffer/queue to prevent partially populated data from being transferred to the PPU.
+ *
+ */
 
 #ifndef SNES_INTERRUPT_INCLUDE
 #define SNES_INTERRUPT_INCLUDE
 
 #include <snes/snestypes.h>
 
+/**
+ * \brief VBlank ISR flag.
+ *
+ * Used to detect lag-frames in the VBlank ISR.
+ *
+ * This variable is set to a truthy (non-zero) value in WaitForVBlank()
+ * and cleared in the NMI ISR after the call to nmi_handler.
+ *
+ * vblank_flag can be used in a custom nmi_handler to detect lag frames.  Within nmi_handler:
+ *  - If vblank_flag is truthy (non-zero), the nmi_handler was called at the end of the frame.
+ *  - If vblank_flag is 0, the nmi_handler was called in the middle of a lag frame.
+ *
+ * \b CAUTION: This variable SHOULD NOT be changed outside of WaitForVBlank()
+ */
+extern u8 vblank_flag;
+
+/**
+ * \brief Lag-frame counter.
+ *
+ * This variable is incremented on every VBlank Interrupt that occurs during a lag-frame.
+ *
+ * \b CAUTION: The lag frame counter cannot tell the difference between a lag-frame and
+ * force-blank setup loading graphics to the PPU.
+ *
+ * \c lag_frame_counter can be modified.  This is useful in development builds to measure the
+ * amount of lag in a level by resetting \c lag_frame_counter on level load and printing
+ * \c lag_frame_counter on a pause screen or at the end of the level.
+ */
+extern u16 lag_frame_counter;
+
+/**
+ * \brief VBlank routine
+ *
+ * This function is called on \b every VBlank interrupt by the \ref VBlank-ISR "VBlank Interrupt Service Routine".
+ *
+ * \b CAUTION: Writes to \c nmi_handler are <b>not atomic</b> and can cause a crash if a VBlank
+ * Interrupt occurs in the middle of the \c nmi_handler write.  Use nmiSet() or disable NMI
+ * interrupts when modifying \c nmi_handler.
+ *
+ * <b>Assembly note:</b> This function pointer will be called with a non-zero Direct Page register
+ * to prevent \c nmi_handler from clobbering the tcc imaginary registers.
+ *
+ * \see \ref VBlank-ISR, nmiSet()
+ */
 extern void *nmi_handler;
 
 /** \brief VBlank NMI Enable  (0=Disable, 1=Enable) (Initially disabled on reset) */
@@ -140,20 +214,30 @@ The H/V-IRQ flag in Bit7 of TIMEUP, Port 4211h gets set when the V-Counter gets
 #define REG_HVBJOY (*(vuint8 *)0x4212)
 
 /**
- *  \brief 
- *      Add a handler for the given interrupt mask.<br>
+ * \brief Sets the #nmi_handler (VBlank routine).
  *
- *      Specify the handler to use for the nmi interrupt.<br>
- *  \param handler 
- *      Address of the function to use as an interrupt service routine<br>
-*/
-#define nmiSet(handler) nmi_handler = handler;
+ * This function will also disable any active IRQ interrupts, enable VBlank interrupts and enable Joypad Auto-Read.
+ *
+ * \param vblankRoutine the function to call on every VBlank (NMI) interrupt.<br/>
+ *
+ * \b CAUTION: \p vblankRoutine is called on \b every VBlank interrupt.
+ * #vblank_flag can be used to determine if \p vblankRoutine was called during a lag-frame.
+ *
+ * \b CAUTION: This function will override the default #nmi_handler.
+ * If you are using consoleDrawText(), you will need to call consoleVblank() inside \p vblankRoutine.
+ *
+ * \see \ref VBlank-ISR, #nmi_handler
+ */
+void nmiSet(void (*vblankRoutine)(void));
 
 /**
- *  \brief 
- *      Wait for vblank interrupt<br>
+ * \brief Waits for a VBlank interrupt.
+ *
+ * Sets the #vblank_flag and pauses execution until the #vblank_flag is cleared (by the \ref VBlank-ISR).
+ *
+ * \b CAUTION: This function will loop forever if VBlank interrupts are disabled.
  *
- *      Waits for a vertical blank interrupt<br>
+ * <b>Assembly note</b>: This function will not modify the A/X/Y registers and can be called with an 8 or 16 bit <tt>.ACCU</tt>/<tt>.INDEX</tt>.
 */
 void WaitForVBlank(void);
 

pvsneslib/include/snes/sprite.h

@@ -109,7 +109,12 @@ Size bit (Small/Large) can be defined in OBSEL Register (Port 2101h).<br>
 */
 extern t_sprites oambuffer[128]; /*!< \brief current sprite buffer for dynamic engine */
 
-extern u8 oamMemory[128 * 4 + 8 * 4]; /*!< \brief to address oam table low and high */
+/**
+ * \brief to address oam table low and high
+ *
+ * This buffer is automatically transferred to the PPU OAM by the \ref VBlank-ISR on non lag-frames.
+ */
+extern u8 oamMemory[128 * 4 + 8 * 4];
 
 /*! \def REG_OBSEL
     \brief Object Size and Object Base (W)

pvsneslib/include/snes/video.h

@@ -337,6 +337,8 @@ void setMode(u8 mode, u8 size);
 
 /*! \fn  setScreenOn(void)
     \brief Put screen On.
+
+    Calls WaitForVBlank() before enabling the screen to flush VBlank buffers/queues and minimise glitches.
 */
 void setScreenOn(void);
 

pvsneslib/source/consoles.asm

@@ -418,11 +418,12 @@ consoleVblank:
 consoleInit:
     php
 
+    sep #$20
+
+    lda.b #0                                                  ; Disable interrupts to prevent the VBlank ISR
+    sta.l REG_NMITIMEN                                        ; from modifying VRAM, PPU Registers or variables.
+
     rep #$20
-    lda.w #:consoleVblank                                     ; Put current handler to our function
-    sta.l nmi_handler + 2
-    lda.w #consoleVblank
-    sta.l nmi_handler
 
     lda.w #0                                                  ; Begin counting vblank
     sta.w snes_vblank_count
@@ -496,10 +497,14 @@ consoleInit:
     lda #TXT_VRAMOFFSET
     sta txt_vram_offset
 
-    plb
+    ; Set nmi_handler, enable VBlank interrupts, enable joypad auto-read.
+    pea :consoleVblank
+    pea consoleVblank
+    jsl nmiSet
+    pla
+    pla
 
-    lda #INT_VBLENABLE | INT_JOYPAD_ENABLE                    ; enable NMI, enable autojoy
-    sta.l REG_NMITIMEN
+    plb
 
     plp
     rtl