byuu's message board

For discussion of projects related to www.byuu.org/


two patches for higan v094 
Author Message

Joined: Sat 14 Jun 2014, 07:00:03

Posts: 4
Post two patches for higan v094
hi ppls

i'm a completely new here but i've been working with higan as i've been learning SNES dev and developing my game (developer blog to come soon).

i've found a couple bugs in higan that no one else has seemed to notice so i thought i'd post patches here.

the first one is a bug in DSP::gaussian_interpolate():

Code:
--- ../../../higan_v094-source/sfc/dsp/gaussian.cpp  2013-05-05 00:22:59.000000000 -0700
+++ gaussian.cpp        2014-08-08 12:19:09.000000000 -0700
@@ -46,7 +46,7 @@
   output  = (fwd[  0] * v.buffer[offset + 0]) >> 11;
   output += (fwd[256] * v.buffer[offset + 1]) >> 11;
   output += (rev[256] * v.buffer[offset + 2]) >> 11;
-  output  = (int16)output;
+  output  = sclamp<16>(output);
   output += (rev[  0] * v.buffer[offset + 3]) >> 11;
   return sclamp<16>(output) & ~1;
}

without this change, if output is <-2^15 at that point the value will incorrectly wrap around. not sure why there was a cast right in the middle of the filter in the first place but i suspect the clamp is actually what was intended.

the second one is in PPU::enter():

Code:
--- /../../../higan_v094-source/sfc/ppu/ppu.cpp       2013-11-08 21:39:08.000000000 -0800
+++ ppu.cpp     2014-08-08 12:23:07.000000000 -0700
@@ -39,7 +39,7 @@
     bg3.begin();
     bg4.begin();

-    if(vcounter() <= 239) {
+    if(vcounter() < (!overscan() ? 225 : 240)) {
       for(signed pixel = -7; pixel <= 255; pixel++) {
         bg1.run(1);
         bg2.run(1);

i'm actually not 100% sure about this one but it seems like a LLE bug. byuu, what do you think?

higan is an amazing piece of software and i'm super happy that it's evolved to take on c++11 features.

btw, does anyone have any tips for debugging SNES games? if i had something like GDB/LLDB for SNES my productivity would 10x what it is now.

Fri 08 Aug 2014, 20:35:52

Joined: Fri 10 Apr 2009, 15:00:08

Posts: 13668
Post Re: two patches for higan v094
As for the DSP, I do realize that value can overflow. But in this case, it's a clone of blargg's code, so we'd have to ask him. In truth, I don't know why we are even doing the cast (or if you're right, clamp) there at all. But without proof it's wrong, I'm going to trust blargg. The PPU mode 7 calculations have been shown to use similar sign-extend (which is what an int16 cast is) functionality, so it's not entirely unreasonable to think that's what the SNES does.

Code:
inline int SPC_DSP::interpolate( voice_t const* v )
{
   // Make pointers into gaussian based on fractional position between samples
   int offset = v->interp_pos >> 4 & 0xFF;
   short const* fwd = gauss + 255 - offset;
   short const* rev = gauss       + offset; // mirror left half of gaussian
   
   int const* in = &v->buf [(v->interp_pos >> 12) + v->buf_pos];
   int out;
   out  = (fwd [  0] * in [0]) >> 11;
   out += (fwd [256] * in [1]) >> 11;
   out += (rev [256] * in [2]) >> 11;
   out = (int16_t) out;
   out += (rev [  0] * in [3]) >> 11;
   
   CLAMP16( out );
   out &= ~1;
   return out;
}


Sorry that my memory's fuzzy, but when does the gaussian interpolation take place? Before or after the samples are written to the echo buffer? I thought it was before, and if so, I'd be surprised if blargg's DSP test program wouldn't have caught this issue, but it's possible.

As for the PPU ... it's due to a partial emulation. Technically, overscan can be turned on and off during lines 225-239, and it takes effect (almost) immediately. Since I don't know what runs and what doesn't, I simply run the PPU anyway, and blank out its pixels in screen.cpp if it's not on.

This obviously has a very slight performance slowdown, but it won't affect accuracy since the PPU rendering is mostly read-only, and the few parts that aren't check the overscan bit (eg OAM address reload.) However, it is something that I need to improve once I get to trying to observe the effects of toggling overscan during this window.

Sun 10 Aug 2014, 19:42:24
User avatar

Joined: Fri 10 Apr 2009, 20:54:19

Posts: 2679
Post Re: two patches for higan v094
It's supposed to overflow like that. Various samples, such as the Square games' wind noise sample, and A Link to the Past's sword click sound (stab a wall, spike blocks bouncing off walls, etc) depend on it to sound correct.

EDIT: My bad, wrong overflow. But still, this is designed to mimic verified hardware behavior.

_________________
blag

Tue 12 Aug 2014, 01:28:52

Joined: Sat 14 Jun 2014, 07:00:03

Posts: 4
Post Re: two patches for higan v094
Yeah oops nevermind, it's probably right given it's so intentional. Just came as a shock since I didn't read any documentation on this when learning about the DSP. kode54, is there a link with more info about this hardware quirk? I'll also try to reproduce on real hardware sometime around the end of the month and report back, for completeness.

Thanks for the PPU code explanation. I agree it probably does no harm but thought I'd point it out anyway in case a refactor or something happens in the future.

SNES debugger??? I've been breaking on SPC700::op_step() / CPU::op_step() in LLDB. It works but :cry: If there is existing code, I'd love to alpha test (and potentially help extend) otherwise I may just write my own little personal hack.

Tue 12 Aug 2014, 08:16:23

Joined: Fri 10 Apr 2009, 15:00:08

Posts: 13668
Post Re: two patches for higan v094
> Just came as a shock since I didn't read any documentation on this when learning about the DSP.

Honestly, this is my fault. I should comment code that looks suspicious but is confirmed on real hardware as correct.

And more importantly, I should definitely be flagging code I haven't verified. But ... yeah. Let me just find a way to stop time for a few years and I'll get that together :P

> I agree it probably does no harm but thought I'd point it out anyway in case a refactor or something happens in the future.

Sure, thanks for looking at the code at any rate. It's a huge help when people do that. Even with a few false positives, it's good to keep me thinking about the code, and we may just catch something serious.

> SNES debugger??? I've been breaking on SPC700::op_step() / CPU::op_step() in LLDB. It works but :cry: If there is existing code, I'd love to alpha test (and potentially help extend) otherwise I may just write my own little personal hack.

It only works on GTK+, and even then ... the latest WIP is higan v094r08 from January.

There's been so many changes to the UI toolkit (phoenix), that it's probably going to be a week-long project before I can even produce a new WIP.

But, if you have Linux and want to mess around with GTK+, and don't mind making game folders and then running loki (my new debugger) on the command-line, let me know. It's console-driven, but I really like it a lot more than any other SNES debugger I've ever used. I imagine that won't be the case for most, who prefer UIs.

I won't be able to help you with any bugs for quite a while, though.

Tue 12 Aug 2014, 09:23:44

Joined: Sat 14 Jun 2014, 07:00:03

Posts: 4
Post Re: two patches for higan v094
Quote:
Sure, thanks for looking at the code at any rate. It's a huge help when people do that. Even with a few false positives, it's good to keep me thinking about the code, and we may just catch something serious.


Yeah higan has been an invaluable tool in helping me learn to program the SNES. I've read all the classic SNES docs and the newer wiki sites that exist but they leave out a lot of corner cases and nothing is more exact than higan. The higan codebase is not a hard one to become familiar with and I still have a bunch more to learn so I'll keep you posted if something else comes up.

Quote:
But, if you have Linux and want to mess around with GTK+, and don't mind making game folders and then running loki (my new debugger) on the command-line, let me know. It's console-driven, but I really like it a lot more than any other SNES debugger I've ever used. I imagine that won't be the case for most, who prefer UIs.


Hehe, like I said, something like GDB/LLDB would be ideal for me. Why is there a GTK+ dependency if it's command-line based? I'm guessing it's like higan in that from the GUI it asks for a ROM then you bring up a terminal emulator that you wrote that backends to the command line core. I'll probably write code so I could do something like this straight from the UNIX command line:

Code:
$ higandb ~/snes/test/test.smc
(higandb) break set -p SMP *0x8000
Breakpoint at instruction address 0x8000 (on the SMP coprocessor)
(higandb) run
Break at instruction address 0x8000!
> mov a, y
(higandb) registers
a: $00
y: $00
x: $00
....
sp: $fe
(higandb) continue
Break at instruction address 0x8000!
> mov a, y
(higandb) quit
$


In any case, the most important thing is that there is existing code so I don't have to start from scratch, the exact syntax doesn't matter. I don't want to fork or anything either, I just need a tool to help me dev faster!!! I'll send you patches for the stuff that I find useful but otherwise not looking to direct the project in any direction. And yeah, I don't mind GTK or Cocoa or Win32 or wxWidgets or QT (some background on me, I wrote this: http://getsafe.org/about).

Tue 12 Aug 2014, 18:13:20

Joined: Fri 10 Apr 2009, 15:00:08

Posts: 13668
Post Re: two patches for higan v094
> Why is there a GTK+ dependency if it's command-line based?

I implemented my own terminal widget, as I wanted finer-grained control over the output window, and I also wanted to add some special key bindings that were difficult with eg gnome-terminal, and probably most importantly ... so that it -can- run on eg Windows in the future, once I port the Console widget I created.

This is roughly what it looks like: http://byuu.org/images/loki/loki-20140130.png
Feature list is here: viewtopic.php?f=16&t=4555

> And yeah, I don't mind GTK or Cocoa or Win32 or wxWidgets or QT

Oh good, then you'll love higan and loki. They are written with phoenix =)

(phoenix looks like Qt, but with C++11 features like lambdas, and without all the pointers, and no preprocessor. It has backends that wrap Win32+GTK+Qt+Cocoa. I would do wxWidgets, but that seems rather ... meta.)

Tue 12 Aug 2014, 20:53:54

Who is online

Users browsing this forum: No registered users and 0 guests

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum