Add Object Removal Code GUI

[cegli2]

This is a feature that has been added to Dolphin VR for a while now, that I thought may be useful in the main branch, (especially now that SBS 3D has been added). It lets users remove any object from being rendered. It's useful for removing unwanted effects (e.g. "heat wave" effect in SMS), or removing objects that don't render in VR/Stereo correctly. Fake 16:9 bars are a good example of this.

Object removal codes can be found by using the fifoplayer, or by brute forcing during gameplay. To brute force a code, start with an 8-bit code and keep clicking up until the object/effect disappears. Then click 16-bit code, add two zeros to the right side of the code and click up again until the object disappears again. Keep doing this until the code is long enough to be unique.

Codes can be disabled or enabled, and included with releases, just like AR codes. I'm new to C++, dolphin, open source projects, and git/github, so let me know if anything doesn't follow proper procedures or needs changing.

[phire]

@dolphin-emu-bot rebuild

[neobrain]

Can we have some screenshots? :)

[JMC47]

On a scale of 1 to 10, this sounds badass.

Looking forward to testing it and figuring out how it works.

[cegli2]

Here's an example of the GUI:

Here's before:

Here's after:

[phire]

I almost think it would be better to do a free-form text entry box, rather than selecting between the possible lengths and then automatically detect the length.

Instead of storing it at a u64, store it as a length plus a variable length array of U8s

This will avoid any problems with leading 0 bytes.

[magumagu]

@dolphin-emu-bot rebuild.

Does a binary comparison on vertex loader input really work well enough in practice? You could very easily end up with UI elements that can't be removed reliably without breaking other stuff (because the vertices are identical to something else, because it doesn't use consistent vertex data, because multiple relevant objects are drawn with a single draw call). How does this compare to other approaches, like using custom textures, or patching the game? I'm not sure it's a good idea to go in the direction of adding more ways of messing with graphics output from within the video backend; projection hacks were a complete disaster.

Please don't submit PRs with merge commits; we prefer rebasing.

In VertexLoaderManager, please use std::equal or something like that; that loop is very difficult to read.

I haven't reviewed the UI code.

[cegli2]

@magumagu I was a bit worried about that too, but surprisingly, it works quite well. I've found so far that objects are consistent, and a single code works for the entire game. One exception would be something like Viewtiful Joe, where the 16:9 bars are a scrolling animation. There many codes have to be added to completely remove the scrolling animation. Situations like this are rare though. 128-bits is enough to find a unique comparison about 99% of the time, which means it doesn't remove anything else. I would also assume that no object codes would be enabled by default, just like the AR codes. They would be there for users who are more into hacking the game, or people using S3D/VR.

Compared to using custom textures or patching the game, these codes can be found by a user with no technical knowledge in typically under 5 minutes for basically any object. We've been using it in the VR branch, where there many objects/effects that users want to remove, and they've been able to find their own codes to fix their issues. It's also easy for us to find 16:9 bar removal codes, taking only a couple minutes per game.

[RachelBryk]

For another use case, this is extremely interesting for tasing, to get rid of objects that obfuscate the user's view of things they need to see.

[cegli2]

@phire About the free form textbox: I thought about that a bit, but thought it might be confusing because leading and trailing zeros are actually valid data. So if you put in 0x0000DEADBEEF0000 it would have to save the entire thing, not 0xDEADBEEF0000. I thought this might be confusing to users, and also would make the UP/DOWN buttons that are needed for brute-forcing hard to code. All of the GUI code would be a lot trickier/sloppier like that too. Would probably have to be hacked together like the 2x 32-bit values I use in a lot of the GUI parts.

[phire]

Leading zeros are only a problem if you treat it as an int, which you have already discovered causes issues. You run into the same kind of issues when you treat telephone numbers as ints.

It's not a number, it's a variable length array of U8s.

So with the search procedure, we are expecting users to step through every single hex value until they find one which works?

It's not exactly pretty, but I guess the alternative option is to build a full graphics debugger gui.

[cegli2]

With the search procedure, the user can do the following: Lets say the object code is 0x442100EA.. They start at 00 and hit the up button until it gets to 0x44 and they see the object disappear. Now they click the 16-bit code and start at 0x4400 and go up until they get to 0x4421 and see the object disappear again. Now 24-bit and they'll see it disappear instantly, then finally they will do go up to the 32-bit code and start with 0x44210000 and have to hit up all the way to 0x442100EA, where they find the final 32-bit code. In this case the user would have to hit the "Up" button 335 times, which isn't terrible for something that only has to be found once.

Ideally, the commonly requested ones to be disabled will just be included in Dolphin's .ini's (like AR codes), so the user doesn't have to search for them at all. You can also take a fifoplayer dump and just look at the code there, but I usually find that takes longer than just bruteforcing it by rapidly hitting the up button and watching the screen.

[BhaaLseN]

Would it be feasible to do something similar to the "overlay texture format" debugging switch for those objects?

[CarlKenner]

Wouldn't it make more sense to do this per nibble rather than per byte?
To find a 64 bit code, you would have to press the button one thousand times if you do it per byte, but only one hundred times if you do it per nibble. If you did it per bit you would only have to press the button 30 times.

[cegli2]

@CarlKenner Yeah, I thought about that. It would definitely be better for brute forcing, but the DataReader class is setup to use u8's, and I bet there would be a significant penalty to render speed if it was comparing nibbles all the time. There would have to be twice as many loops through the code to match/rule out the data if it was done with nibbles.

I guess there could be a mode for brute forcing that compares nibbles, and a mode that compares u8s, but it would be a lot of extra code/bloat. Trade-offs...

[CarlKenner]

Good point

[magumagu]

You can just pack the nibbles, construct an appropriate mask, and do "(input & mask) == value". Actually, that's probably faster than comparing byte-by-byte.

That also simplifies the dialog: the length of the code is just 4*strlen(input), so there isn't any need to explicitly select the size.

[cegli2]

@magumagu Having a mask like that would get around the DataReader u8 issue, but you'd still have to go through the matching loop

for (int d = 0; d < m_LocalCoreStartupParameter.num_object_skip_data_bytes[current_object_removal_code]; d++)

twice as many times as if you compared u8s, assuming you wanted to be able to break on a nibble. I'd think that'd be much slower. Maybe I'm misunderstanding something?

[mimimi085181]

What about skipping numbers, if there's no object with that number? So if you are at 0x2100 "size 16 bits", and you press up, it checks if there's something with 0x2101, 0x2102 and so on, until it finds an object to hide?

I think this could speed it up dramatically.

[cegli2]

@mimimi085181 That's a pretty cool idea. The code would have to be smarter than it is right now, because right now it has no concept of where it is in a frame or how many objects total are being rendered. It'd have to know when a frame started and ended so it would know when all the possible objects had been tried with no matches. It'd add a small speed penalty when you had the codes enabled, but it would definitely speed up the brute-forcing process. Might be an interesting future enhancement to try!

One of things I really tried to work on in the vertexmanager code was to get it as fast as possible, since it is called so often. I tried to make it as fast as possible (programmed it a couple different ways and checked out the assembly to judge the speed), because every instruction saved really counts when it's called so many times per frame.

[magumagu]

You missed the "pack the nibbles" part: you can do for([...]){if ((PeekU64() & mask) != data) break;} or something like that. That allows specifying the length of the code in bits, and it's just one comparison for codes up to 64 bits, two comparisons for 128-bit codes.

[cegli2]

@magumagu Ooh, I see what you're saying now. Hmm, yes that is interesting... I do agree that would probably be faster. Might even be worth it to start by packing the u8s and creating a mask field just for the speed up. Nibble support could then be added after that is complete...

[degasus]

This PR sounds for me to share the same issues as custom geometry has: How to identify the geomtry.
First, to get more unique values with just some bits, use a hash on the whole input data size, so u64 is by far unique enough to not get collisions at all.

But, hashing the vertex loader input has some issues: Games usually render indiced. So the real geometry won't be hashed, just the indices. This means, that two completely unrelated geometry which just have the same amount of primitives may conflict. (ok, tbh, likely only if also the graph of the shared vertices matches, but however...) But this can be fixed by hashing the output of the vertex loader. Too bad, this is both a small performance issue and not allowed on the current ogl design, but it seems to already work fine for z-freeze ...

But there is a much bigger issue right now: How to seperate between identical geometrys but different objects? I think this won't matter for 3d objects, but what about fullscreen (or full viewport) quads? I don't see any nice way to identify this objects. Maybe also include the viewport or the texture configuration into the hash? ...

[delroth]

In general that sounds to me like a bad way to implement this. As @degasus said, this will have false positives in many cases. If you're going to do custom "object removal codes" for each game, why not just implement this as AR codes anyway?

Maybe we need to provide some better utilities for code patching in Dolphin (that does fuzzy matching on code).

[cegli]

@delroth @degasus I agree it's possible that there could be overlap (especially with short codes), but with a long code I haven't had any issues or problems in the games I've played through. I'm also worried about the performance issues of hashing. Identical geometry is a limitation, though my use-case for this was focused on HUD issues in 3D/VR, odd effects that some users don't find desirable and don't render in 3D/VR correctly (heat wave, etc), and 16:9 bars, so this hasn't been an issue for that. It depends on what the user wants to achieve I guess.

AR codes are way harder to find, and require a lot of technical knowledge. There's no way I (or most users) would be able to find an AR code to remove an object in a couple minutes. Is there a way to quickly brute-force an AR code for an object that I'm unaware of? My game-hacking knowledge is fairly limited.

It's not perfect, and it might not cover every single person's use case 100%, but it's been a huge help for our VR niche. I don't have a 3D display, but I bet it'd really be useful there too. I definitely favored code speed over features/perfect accuracy. I felt that was a good decision, but I guess that's just my opinion :).

Thanks everyone so far for your suggestions. I'm going to work on implementing some of them when I have time. Even if this PR doesn't make it into the main Dolphin branch, a lot of them will be put to use in the VR version!

[delroth]

At this point, I think the state of this PR is the following:

1. Most of the code is not in compliance with our coding style.
2. Some of the design of this code is weird — hardcoding code sizes for example where instead a vector of nibbles or bytes could be used to make the whole thing both more generic and simpler.
3. It's not clear if we even want this feature or not in Dolphin. It has dubious uses right now while introducing a significant complexity, as well as more ways for stupid users to shoot themselves in the foot.

As such I will be closing it now. Don't take that as a definitive no on the feature, more like a disagreement on the current implementation (+ some work to be done on convincing).

[cegli]

@delroth Fair enough on point two. It would be cool to change some of it to have nibble support for faster brute forcing some time.

For point one, I'm a bit confused because almost all of the code came from the patch engine and action replay tab. I modified it slightly from there, and also fixed everything that was pointed out. I know there's a hacky section or two (validating the up and down buttons when wxWidgets can only handle a 64bit number for instance), but I thought most of it was pretty close by now. Are there specific things I should look at in the future?

For number 3, I think most of the demand would be from stereoscopic 3D users. In the first stereoscopic 3d thread, two users requested this in the first page of posts (https://forums.dolphin-emu.org/Thread-stereoscopic-3d), which is why I decided to make a PR for it. It's almost a necessity for VR, if you ever officially plan on supporting that as well. It's true that it adds complexity, and it's amazing what users can manage to mess up, but I'd argue it's no more risky than the patch engine or AR code tabs right next to it :).

Thanks for taking the time to look it over! I've ported all of the changes over to Dolphin VR, and the code is a lot faster and nicer now, which is great, so something still came out of this PR either way.

[cammelspit]

+1

[lioncash]

@cegli I can't speak for all of the points, but I can give advice with the wx one.

With regards to handling the up-down in wx, it would be best to make a subclass of that control and internally handle the limits, rather than letting the containing class deal with it. This way, if you ever need to move controls around, you don't need to shuffle any code around; you can just "declare and be done with it".

[cegli]

@lioncash That's a good idea, thanks for the advice!

[lioncash]

No prob, glad to help :)