Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

differences from upstream #196

Open
3 of 13 tasks
dantob opened this issue Aug 16, 2020 · 6 comments
Open
3 of 13 tasks

differences from upstream #196

dantob opened this issue Aug 16, 2020 · 6 comments

Comments

@dantob
Copy link

dantob commented Aug 16, 2020

this is some comparisons between mgba 0.8.3 and mgba-libretro 0.8.3

commits downstream only

  • Add savetype overrides for e-Reader f8cc284
  • Fix missing include. 27befce
  • Fix libretro cmake support d7758dc
  • Libretro: Add Italian core options translation 5a4530f
  • Add optional interframe blending d415cca
  • Add 'fast' variants of interframe blending methods 0f797cc

commits downstream only, but only partially

  • Fix issues with PS3 build (src/core/thread.c) ebe14cb
  • Add PS2 support 7279b36
  • Libretro changes (src/platform/posix/memory.c) e16338f
  • Start making it work for Android (src/util/crc32.c, probably should remove this change) d98d930

files renamed

  • src/core/serialize -> src/core/core-serialize.c
  • src/gba/hle-bios.s -> src/gba/hle-bios-src.s

differ from upstream, but cant find specific commit

  • src/gba/sio.c differs
@endrift
Copy link
Collaborator

endrift commented Aug 16, 2020

So, is this a bug? I'm not sure what this issue is trying to represent.

@dantob
Copy link
Author

dantob commented Aug 17, 2020

Is it not desirable to merge changes back to upstream, or drop them from the libretro tree if they are no longer relevant? This way we also have 'bug compatibility', and likely much less work to merge for new release. I think you are the main contributor upstream, so ultimately it is your call.

@endrift
Copy link
Collaborator

endrift commented Aug 29, 2020

I've merged some commits, however I'm opposed to many of the downstream changes:

  • Color correction. Slow to do in the core, should be done on the frontend where it can be accelerated e.g. via GPU shaders. Providing gamma values that let the frontend know how it should be adjusted via an API change could be a good idea.
  • Interframe blending. Same complaint as above.
  • "Turbo buttons". Also belong in the frontend.
  • Platform-specific features like the Switch-specific gyro support. Should be part of the API and handled by the frontend. Again.
  • The memory.c file that's just all the rest of the memory.c files merged. I complained about this years ago, but "libretro does things differently", which for some reason involves duplicating code (and bugs; I have fixed bugs in the files that got slammed into that one after all).
  • PS2 support hacks. I complained when the PR was filed because it erroneously changed how color handling was done, but it got merged anyway. Because "more platforms" (that it can barely run on) is somehow trumps correctness and code quality.
  • The file renamings. Only necessary for some random build systems I don't support. Works fine on most platforms it seems!
  • That "missing" include got stuffed into the wrong place.
  • In general, the rubber-stamped merges don't really care about code quality or my code conventions at all, so without me manually stepping in and fixing things every single time, the code rots.

I would be okay upstreaming some more stuff but there are some serious project management issues with how this downstream is handled overall, so the distance keeps growing.

@hizzlekizzle
Copy link

Would it make sense / be preferable to have master/main track upstream and then use branches for that stuff? I agree that color correction, interframe blending and turbo/macros belong in the frontend, but mGBA runs well on a number of platforms that can't support shaders (I know jdgleaver uses it on 3DS quite a bit). OTOH, RetroArch's video filters could be another option there, though it can only load one at a time, so there would need to be a combined version to get both effects.

Turbo in the core is bad if we ever get the 4-player netplay hooked up, as it causes desyncs if not everyone has it enabled, which is why FBNeo removed its macros, IIRC.

@endrift
Copy link
Collaborator

endrift commented Aug 31, 2020

Using branches would make merging less painful (and I know there's been a lot of pain with that before), but then there's the issue of which branch the buildbot is cut from.

Also saying it runs well on the 3DS is...an exaggeration. It barely runs full speed, if it even does, in most games. Adding more video processing on top of that would definitely drag down the framerate. Which is why I don't want this in the core.

@dantob
Copy link
Author

dantob commented Sep 1, 2020

I would set up a new branch (upstream?) that tracks the latest upstream release. and then for each new release something like
git checkout master
git rebase --interactive upstream
to reapply the libretro commits on top, it does mean though that rebased commit ids will change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants