DolphinQt / VideoCommon: Add additional texture dumping options

[iwubcode]

Specifically, this enables users to choose whether to dump mip maps, or base level textures.

[iwubcode]

I expect the UI is likely going to need some changes. But functionally, it should work! Set as a draft until the texture pack creators are happy with it.

[frozenwings1]

I'm very happy with it. Thank you so much!

In terms of cleaning up the UI, you probably don't need a "Dump Base Textures" option, since I don't think anyone is going to be dumping mipmaps without the base textures. Although maybe there are some situations that I currently haven't been in. And like you said, might as well have "Dump Arbitrary Textures" turn off in the UI if you aren't dumping the other ones.

One thing, does the "Dump Arbitrary Textures" option make the "Arbitrary Mipmap Detection" option in the Enhancements tab redundant? It seems like it won't dump arbitrary textures unless you have both turned on currently. I'm not sure if the new option is accomplishing the same thing performance wise as the older option. I hope I'm explaining that well enough. I don't know whether or not it would be wise to somehow link those too options as well, assuming they aren't doing exactly the same thing.

[iwubcode]

Glad you were able to test it! Thank you for your feedback.

In terms of cleaning up the UI, you probably don't need a "Dump Base Textures" option, since I don't think anyone is going to be dumping mipmaps without the base textures. Although maybe there are some situations that I currently haven't been in.

I guess I was thinking of rare cases where you'd already dumped the base textures but didn't dump the mips. Then you realized something and wanted to go back and dump the mips. I mostly added it for completeness.

And like you said, might as well have "Dump Arbitrary Textures" turn off in the UI if you aren't dumping the other ones.

Do you think turning it off or just disabling it? I'm not sure what would be clearer or less confusing to the user. I don't know if we have any other settings which forcibly change the state of another setting.

One thing, does the "Dump Arbitrary Textures" option make the "Arbitrary Mipmap Detection" option in the Enhancements tab redundant?

I think we want both. The "Arbitrary Mipmap Detection" enables or disables the logic. This does pass it down to the dump code. However, are there cases where you want the logic but not the dump? That was my thinking. If we keep both, maybe just a comment in the UI saying that it does nothing if that setting is not on?

[frozenwings1]

I guess I was thinking of rare cases where you'd already dumped the base textures but didn't dump the mips. Then you realized something and wanted to go back and dump the mips. I mostly added it for completeness.

Alright, I guess that's a fair point. I don't think that happens very often though, but for completeness it seems like you might as well while you are here working on stuff. I was thinking of saving space in the UI, but I suppose that's not a huge concern compared to wanting an option that doesn't exist.

Do you think turning it off or just disabling it? I'm not sure what would be clearer or less confusing to the user. I don't know if we have any other settings which forcibly change the state of another setting.

I think just doing the same thing that happens if you turn off the "Enable" option in the Texture Dumping section. Just "gray out" the "Dump Arbitrary Textures" option if the other options aren't selected.

I think we want both. The "Arbitrary Mipmap Detection" enables or disables the logic. This does pass it down to the dump code. However, are there cases where you want the logic but not the dump? That was my thinking. If we keep both, maybe just a comment in the UI saying that it does nothing if that setting is not on?

I don't think we need the logic when someone isn't dumping it, but that's just my opinion. I leave the setting turned off for the most part, since in some games it triggers false positives. I don't know the best way to proceed, playing it the safe route would be to just leave a comment like you said.

[iwubcode]

Thanks again for the feedback.

I believe you've convinced me. I think I will remove the Dump Arbitrary Textures option. It seems to make the UI more confusing and there's probably no reason to avoid dumping those if Arbitrary Mipmap Detection is on and the user has turned on dumping. I can't imagine there are many of those textures, so being able to avoid them isn't that valuable.

[frozenwings1]

Yeah they only make a difference in niche situations. For example, I've been working on the Skies of Arcadia pack off and on for like 3 or 4 years. When I updated my Dolphin I was dumping duplicate textures (textures that were the same as my old ones, but flagged with "arb") because the resolution is so low it was triggering false positives. So I had the same textures twice, one with the arb flag and one without. I ended up using Bighead's texture tool to remove all the flags so I knew which ones were really new and which ones were old. Now I just leave the setting off for the most part unless I know for a fact the game has arbitrary mipmaps.

Someone who is coming in and starting to dump AFTER the option was added won't have any problems at all since having the arb tag for false positives makes no difference unless you have really old files you're comparing them to, such as in my case.

[frozenwings1]

It might be worth moving the "Arbitrary Mipmap Detection" to the new "Texture Dumping" section though. I didn't even know it was an option because I never really go to those tabs very often. Bighead was the one who ended up telling me about it.

[iwubcode]

It might be worth moving the "Arbitrary Mipmap Detection" to the new "Texture Dumping" section though.

So, that option is actually a visual option for users (see #6118 and #6158) It does feed into texture dumping, which is why I originally had the option "Dump Arbitrary Textures". Maybe I can add a comment to the other two toggles to mention that arbitrary textures are also dumped if that option is turned on.

[frozenwings1]

A comment sounds like a good idea. Something like "Automatically dumps arbitrary textures if Arbitrary Mipmap Detection is enabled in Enhancements."

[iwubcode]

@frozenwings1 - thanks again for your help. How does it look now?

[frozenwings1]

@iwubcode Tried out the new build and everything looks and works great! Thanks again for working on this, and especially so quickly.

[Pyrii]

Another pull changed the config number from 97 to 102, thus the conflict. I really want to see this pushed as it would also improve stuttering when dumping and reduce the amount of stuff to sift through.

Is there any need for the original dump toggle though? selecting either or both should be fine to know if you're dumping I'd assume.

[iwubcode]

Thank you @Pyrii , I hadn't noticed this was broken.

Is there any need for the original dump toggle though? selecting either or both should be fine to know if you're dumping I'd assume.

Yeah, maybe not. It's a good point. I kept it mostly for backwards compatibility. I'd be interested to hear what the rest of the team thought.

[frozenwings1]

Yah, I was wondering what the hold up was. I've been using this branch for a couple weeks now, haha. Works like a dream by the way. Thanks again!

[jordan-woyak]

Code looks good. Untested but it looks pretty straightforward.

[Tilka]

What's the use case to have base textures disabled but mip maps enabled? It seems to me all three checkboxes should be merged into a single radio button group (disabled, base only, all mips).

[iwubcode]

@Tilka - nothing in particular. Maybe they dumped the textures, modified those but then later decided they wanted to dump the mips. It's definitely an edge case but I figured I'd give it to them.

A radio button would be fine but I guess with the current approach I could simplify down to just the Base / Mips checkbox (enabled would be if either is checked) which would fit into the two column UI layout a little more nicely.

@jordan-woyak - thank you for reviewing!

[iwubcode]

On IRC we discussed the radio button solution and determined that wasn't feasible given the hotkey toggle capability.

This review should be ready to merge!