nesdev.org

I was just noticing an issue with the NTSC filter in Nestopia.
If you look at the screenshot, there is a color transition edge on the left and right sides of the screen.
However, this should not actually exist, since there should be a little bit of overscan on the left and right sides of the image. The background color should continue a little longer, and should not have any color change at the edge of the screen.

Dwedit wrote:I was just noticing an issue with the NTSC filter in Nestopia.

super_mario_bros_title_screen_nestopia.png

If you look at the screenshot, there is a color transition edge on the left and right sides of the screen.
However, this should not actually exist, since there should be a little bit of overscan on the left and right sides of the image. The background color should continue a little longer, and should not have any color change at the edge of the screen.

Sega Li can display background color and its NTSC filter looks great ...
but it can't use scanlines..

This is because of the instances of lut.black in ./source/core/NstVideoFilterNtsc.cpp. If they were replaced by the appropriate palette numbers, it would DTRT. Unfortuantely, the C++ goo is all over nestopia and I haven't least idea how to properly violate the abstraction to get the correct palette data.

I attempted to get through the abstractions to pass the information from the palette to the NTSC filter. I added three bgColor members to let it trickle down. It looks like it could also have been passed partially through the Blit() call, as the burstPhase is. But adding a parameter to Blit() would have involved changing all the renderers' Blit() functions. I don't have the Linux version to try compiling, so you'll have to try these changes to the source.

This is kind of hacky, and still isn't enough. Ideally, Nestopia would pad the left and right sides of the raw PPU output with zero-pixel colors, so that the renderer could show some overscan (if desired) on the left and right sides, and get even more of the background color. As it is, it just gets a few pixels of it (that are mostly cut off) as necessary to render the pixels properly in NTSC mode.

> I attempted to get through the abstractions to pass the information from the palette to the NTSC filter. I added three bgColor members to let it trickle down.

Hah, that reminds me of working with libgambatte. I want to get the Y-counter for the LCD renderer, that the SGB emulation needs. LCDC is a private class that doesn't expose it. Itself only a private member of the LCD class. The LCD class is only a private member of the Memory class (which doesn't really make sense, but whatever.) The Memory class is only a private member of the CPU class. The CPU class is only a private member of the GambatteCore class. The GambatteCore class is only a private member of the libgambatte interface class.

The actual Y-counter value was -extremely- well guarded from the outside world. I'd go crazy trying to write a debugger like that.

With my code, I made up a keyword called privileged. When you compile with -DDEBUGGER (which has no speed penalty), it becomes public. When you compile without that, it becomes private. This helps keep the interface clean and guarded, and also lets you get at the data you want when you absolutely need it.

It also helps to design the core where you have system{CPU, APU, PPU, Memory} rather than system{CPU}->{APU}->{PPU}->{Memory}. All one level deep rather than all nested inside each other.

I did something like that too. The value in access control is enforced restrictions on users, making modifications easier since you can better predict the consequences of a change to the class. By being able to enforce this for normal builds, you get that value, but can then take it away where full access is desired. If friendship were transitive, you could just make a base class a friend and derive from that for special access. Nested classes are tedious to deal with as well; better to put them into a side namespace and include its header as needed.

Surely there's a balance between overly layered systems and everything-is-a-global ones that are a nightmare to modify without breaking (but easy to write debuggers for). Both suffer from major resistance to certain kinds of changes, a kind of momentum that ties the hands of maintainers in different ways.

A huge advantage would be like you said, generic friends.

People love real life analogies with computing, so they'll see "a friend of a friend may not be your friend", and that "only the class itself can declare who its friends are", sure, but when it comes to programming, it'd be nice to have something like a debugger class that says it is friends with your emulator core classes. Or a single Emulator::Debugger friend class you could inherit from and get the desired access. The users knows when they declare these things that it may break in a future library revision. Private access is trivial to bypass anyway (byuu.org/articles/programming/public_cast)

Easy way to do the latter would be to extend the keyword. friend == private friend, and then add protected friend and public friend.

At this point, I want subclasses so badly I'm seriously contemplating a custom preprocessor

blargg wrote:I attempted to get through the abstractions to pass the information from the palette to the NTSC filter. I added three bgColor members to let it trickle down. It looks like it could also have been passed partially through the Blit() call, as the burstPhase is. But adding a parameter to Blit() would have involved changing all the renderers' Blit() functions. I don't have the Linux version to try compiling, so you'll have to try these changes to the source.

This is kind of hacky, and still isn't enough. Ideally, Nestopia would pad the left and right sides of the raw PPU output with zero-pixel colors, so that the renderer could show some overscan (if desired) on the left and right sides, and get even more of the background color. As it is, it just gets a few pixels of it (that are mostly cut off) as necessary to render the pixels properly in NTSC mode.

Thanks for taking a look at this, i tried to compile the changes but ended up with errors for some reason but i will try again.

What errors? I didn't compile my changes so it's unlikely they would work without minor errors.

blargg wrote:What errors? I didn't compile my changes so it's unlikely they would work without minor errors.

Sorry blargg i meant i came up with compiling errors about subclass i believe when i tried to compile the changes. Does the code changes in effect pretty much try to do this?:

http://img40.imageshack.us/img40/8381/1nesg.jpg

This pic is of the actual nes, you can see the blue sky continues on the right side of the screen.

Yes, it is to set the background that Nestopia uses to the NES's BG color, rather than black as Nestopia currently uses.

blargg wrote:Yes, it is to set the background that Nestopia uses to the NES's BG color, rather than black as Nestopia currently uses.

Ok thanks ill try to add the changes you posted again and see how it goes. Perhaps i did not do something correctly the first time.

Blargg's patch does not build cleanly (he says so, I've tested). I fought with it for a bit and then gave up...

ok i tried it again and get these errors:

error c2065: 'i' undeclared identifier: NstPPu.Cpp

error c2248: NstMachine.cpp : cannot access private member declared in class ::NES::CORE:PPU

error c2065: bgcolor undeclared identifier NstVideoRenderer

error c2039: bgcolor is not a member of NES::CORE::VIDEO:RENDERER

i made a pastbin of the ntsc.cpp file because i think i messed up something but those are the main errors:

http://pastebin.com/fq8E4F0V

EDIT: ok i see the post before, nevermind lol guess i missed where that was said lol. Ill wait and see if anything else pops up from anyone who might be able to help fix the bug.

OK, I saw that I had since successfully built Nestopia from source on my machine, and got this patch working. I used diff -c6 -r, which I hope allows simple patching (I can re-diff if there's a simpler way). (weird, the sky color looks too purple below, but clicking on the picture shows it right. I'm using Firefox latest version)

[see later post for fixed patch]