mupen64plus / mupen64plus-video-rice

Video plugin for the Mupen64Plus v2.0 project, using OpenGL. This plugin is based on the RiceVideoLinux plugin from earlier versions of Mupen64Plus.
31 stars 40 forks source link

Organize hardware/driver specific values inside the code itself. #42

Open Narann opened 9 years ago

Narann commented 9 years ago

Following this discussion:

@littleguy77 recently introduce a commit exposing a parameter to deal with a known implementation dependant problem: https://github.com/mupen64plus/mupen64plus-video-rice/commit/dc6cc61824368fb458452042e7bc06df2ad2be2a

We will deal like this for now but I'm sure there is a better way:

More I read Dolphin code, more I think this is the way to do. Keeping all those exposed config parameters for very few specific hardware is kinda bloating and let the code with some branching future contributors will have no way to understand. I consider if a value have to be exposed, the code will have to explain clearly in which situation this parameter could be removed.

Keep hardware specific hack inside the code would also be a place for downstream dev to place their own hacks and could possibly merge them with upstream nicely.

littleguy77 commented 9 years ago

I understand about not wanting to expose yet another config param to, say, a Linux user. But that commit was actually the lynch-pin that allowed mupen64plus-ae to converge with upstream. Without it, we'd forever be a fork.

In mupen64plus-ae, we have been exploring this issue literally for years. No practical solution has been found. No elegant solution has been found. It was actually the first issue I ever posted to GitHub, and one of the first issues tracked in mupen64plus-ae: https://github.com/mupen64plus-ae/mupen64plus-ae/issues/5 You see it is still open. As far as I know @twinaphex has not found a clean solution either in his work on the Libretro fork (please correct me if I'm wrong! :) )

The problem is that it's not just a few chipsets or drivers with the issue. The issue affects EVERY mobile device. The correct values vary by chipset vendor and model. If we were developing for iOS, it would be reasonable to enumerate them all, but it's a complete lost cause in Android world. For popular or gaming-specific models (e.g. Google-branded devices, Samsung Galaxy S* models, Nvidia Shield, Xperia Play, etc.) we have taken the time to find the correct values. At run-time, we parse various OS- and Android-provided strings to determine if we are running on one of them, and set the value accordingly. There seem to be broad families with the same values, but it's certainly not a hard and fast rule. For example, the latest Nvidia Shield Tablet requires different values than models with earlier Nvidia chipsets (e.g. Tegra3).

Understand that the devices that are hard-coded here still represent a very small portion of the user base.

The best hope I have for a somewhat clean solution is to do some sort of auto-detection in the core or video plugin at run-time, as is done in OpenGL glide64mk2. But no one has yet taken on that challenge.

Tagging @paulscode @xperia64 @Gillou68310 @Metricity

littleguy77 commented 9 years ago

Also, I think you are only looking at the tip of the iceberg in the Dolphin code. Enumerating a bunch of values and hacks is the easy part (that's what this is). The ugliness and non-portability come from determining which hacks need to be applied. I'd be curious to see where that's done in Dolphin.

One thing I've tried to do throughout the mupen64plus <-> mupen64plus-ae convergence is keep Android out of upstream. Trust me, you do not want native Android code in the upstream repository. Just look at all the Android stuff in SDL2 and you'll see what I mean. Right now the only Android reference in upstream is the logcat API to replace *printf. I'd like to keep it to that.

Narann commented 9 years ago

Thanks a lot, all what you say is highly valuable.

First, I wouldn't blame you for exposing a parameter, it was a true need (and a real OpenGL problem). From my perspective, the point is not (only) to store the correct values but also to simplify hardware specific hacks over the code. This way we "track" hardware specific behavior and can safely remove them in the future when we will no longer support them.

But all of this is not done yet. I was mainly to gather informations here, we have better to do for now. :)