search

libretro / snes9x

Snes9x - Portable Super Nintendo Entertainment System (TM) emulator
http://www.snes9x.com
Other
47 stars 56 forks source link

(Bounty) Enable Blargg NTSC Filters Core Option #186

Closed mr-falafel closed 5 years ago

mr-falafel commented 5 years ago

It is my understanding that this core has/had a built-in core option for Blargg's NTSC Filters but that it is currently disabled. Would it be possible to enable it again?

edit: I have put a $15 bounty on this issue, via Bountysource.

stellarporter commented 5 years ago

Added blargg filter to libretro port. Ready for PR into libretro/snes9x. https://github.com/stellarporter/snes9x/tree/blargg

mr-falafel commented 5 years ago

Awesome! Thank you so much!

mr-falafel commented 5 years ago

How long should it take for this to merge over?

hizzlekizzle commented 5 years ago

Should be closed by https://github.com/libretro/snes9x/pull/206.

With this issue closed, the bounty can be marked as solved and the donors can approve the solution.

mr-falafel commented 5 years ago

Core option is there but the filters are crazy broken! Guess that's why they were disabled...? snes_composite

stellarporter commented 5 years ago

Which OS and game? Tested latest git https://github.com/libretro/snes9x/commit/7103f840a15f01a7dc63ae9fb9b4a9261e5e1b14 on Windows. Tried all filters with shaders and nothing awful happened yet on several games: Donkey Kong Country, Super Metroid, Seiken Densetsu 3, Jurassic Park, Zelda Link to Past, Super Mario World.

hizzlekizzle commented 5 years ago

I'm getting in linux x86_64, as well: Screenshot from 2019-03-20 17-56-19 Just to be sure, I ran a self-compiled lib and the one from the buildbot. Same behavior on both.

stellarporter commented 5 years ago

Changed browser and now I can the KDL3 and 240p screenshot. That heavy green tint suggests a 555 - 565 conversion error somewhere. Why I'm mysteriously not affected is (..?)

I'll give everything another lookover and see if I can bash something out of this.

ghost commented 5 years ago

Screenshot from 2019-03-21 08-48-33

i believe that extra options (sound filters, gfx shaders etc ) if possible should be done in frontend through shaders etc unless they are part of original hardware or emulates what the hardware actually do or have or are actually unique. imho

stellarporter commented 5 years ago

Supposedly some Retroarch ports don't support gpu shaders? Thought I remember someone request a crop feature for C64 VICE core, like how the NES ones do it.

I'm personally neutral on this subject though and willing to fall on either end of the blade. Guidance from thread author and repo owners?

inactive123 commented 5 years ago

I think the blargg NTSC filters added could definitely have value, so yeah, I don't have anything inherently against them.

If you have a new PR where the graphical output issues are fixed with blargg NTSC and we can make Emscripten work, then we can merge it without any issues as far as as I"m concerned.

stellarporter commented 5 years ago

I think the problem could be this line: https://github.com/libretro/snes9x/blob/master/filter/snes_ntsc_config.h#L6

Threw these into each code path and compiler says 565 (win32). #pragma message ("blargg 555") #pragma message ("blargg 565")

I expect nearly all ports to be 565 libretro output, but my assumption is blargg library incorrectly compiles with 555 math which whacks out the colors. Could someone test this theory, flip 555 to 565 in that config.h?

If above is true, libretro uses this flag to test for 565 but I'm unsure how to push this into filter/snes_ntsc_config.h https://github.com/libretro/snes9x/blob/master/libretro/libretro.cpp#L994

edit: Flipped 565 to 555 and I can match above 240p screenshot. Best way to fix that file?

stellarporter commented 5 years ago

Going to try changing #if !defined(SNES9X_GTK) && !defined(_WIN32) to #if PIXEL_FORMAT == RGB555.

edit: (undefined) RGB555 = (undefined) RGB565 = 0.

So it should be: #if RED_SHIFT_BITS == 10, with some port.h changes for C90 bool typedef. I'm sure this will work on linux et al now (untested).

I'll news post about emscripten in other thread.

mr-falafel commented 5 years ago

Screenshot from 2019-03-21 08-48-33

i believe that extra options (sound filters, gfx shaders etc ) if possible should be done in frontend through shaders etc unless they are part of original hardware or emulates what the hardware actually do or have or are actually unique. imho

My understanding is that the blargg filters are specifically tailored for each console. I understand that this is somewhat a personal preference but I feel like the emulation is incomplete without the video processing of the original hardware. Unfortunately, Retroarch's blargg video filters don't work very well for me, here in linux land. The filters built in to other emulators, such as Mesen, work perfectly. This is the reason for my request.

mr-falafel commented 5 years ago

Option for Blargg filters in snes9x is now gone again, for me. Is that right?

hizzlekizzle commented 5 years ago

Yes, we reverted it while we look for a good solution to the pixel format issue.

mr-falafel commented 5 years ago

Okay, that makes sense! Sorry for not properly understanding the thread.

mr-falafel commented 5 years ago

Do you think I should start a new issue for the filters being broken on linux?

hizzlekizzle commented 5 years ago

No, I think this is fine. We'll just reopen this one temporarily so it doesn't get lost/forgotten.

mr-falafel commented 5 years ago

That's great, thanks. :)

stellarporter commented 5 years ago

@mr-falafel Tried compiling some binaries for linux. Have no idea if they work but if you're feeling experimental, ideally they should be master git + blargg filter pr. https://ci.appveyor.com/project/stellarporter/snes9x/builds/23436553

mr-falafel commented 5 years ago

Working beautifully for me! kirby_ntsc

mr-falafel commented 5 years ago

Though, I have only tested the 64bit build. I'm on 64bit Linux Mint.

stellarporter commented 5 years ago

Cool. So we know is doing something right. https://github.com/libretro/snes9x/pull/207

appveyor.yml build script

version: 0.1.{build}

shallow_clone: true

image:
- ubuntu1804

before_build:
  - sudo apt-get --assume-yes install gcc-multilib g++-multilib

build_script:
  - gcc -v
  - cd libretro

  - make -f Makefile platform=unix CFLAGS='-m32' CXXFLAGS='-m32' LDFLAGS='-m32'
  - mkdir lib32
  - mv snes9x*.so lib32
  - make clean

  - make -f Makefile platform=unix
  - mkdir lib64
  - mv snes9x*.so lib64

artifacts:
  - path: '**\snes9x*.so'
mr-falafel commented 5 years ago

As soon as this gets merged, I'd be very happy for the bounty to be claimed!

mr-falafel commented 5 years ago

Is it just a matter of waiting, now?

hizzlekizzle commented 5 years ago

Is it in the buildbot builds now? Are all platforms building and functioning with proper colors?

mr-falafel commented 5 years ago

The latest nightly build for windows and linux is from the 22nd of last month. I'm assuming nightly is where I should be looking. I don't know about other platforms but this build still has the filters disabled for linux because they were broken. Stellarporter's fix worked perfectly for me so I'm just waiting for it to go into the buildbot before I mark this as solved.

mr-falafel commented 5 years ago

I can see that there's a new build of snes9x in the buildbot, today. However, upon testing it, the blargg filter option is still hidden.

mr-falafel commented 5 years ago

Still no linux build with the filters enabled. I can see that the fix has been applied (I think) in the commits but I think they still need to be re-enabled, on the linux build. Meaning the option to turn them on is missing from the core options, right now.

hizzlekizzle commented 5 years ago

This should be fixed now. I'm going to close it so @stellarporter can claim the bounty.

mr-falafel commented 5 years ago

Fantastic! I'd be very happy for @stellarporter to claim the bounty, now. :)