CustomTexture: new name format #1885
Conversation
|
@BigheadSMZ have fun testing :D |
|
Just got home and this is taking priority over both shower and supper. I'll report back soon, so excited!! Edit: Thank you thank you thank you thank you thank you! I never imagined a day where this would be a reality, I've anticipated for so long, and now it's here. This change made my night perfect. Did I mention thank you?! So far it seems to be working with all paletted textures. I'll be testing this and dumping for hours to come, but from what it looks like there will probably no issues to report. Edit 2: Been redumping and I must say I love this. I already eliminated over 96k duplicate textures... Some behavior I noticed is hash_8 textures are also affected (I knew hash_9 was paletted but never knew hash_8 was?), also not every single texture is affected. Some paletted textures keep their ID, both _8's and _9's, which is strange in some circumstances. Examples: Outside of that being strange to me because I assume they are from the same palette, everything seems to be working perfect. I have not stumbled onto a single duplicate texture. |
|
@BigheadSMZ : |
|
@BigheadSMZ Please don't use edits to update your post after a longer time. If there was a progress, just repost so I'll notice ;) btw, the _10 format is still broken a lot. The code doesn't calculate the tlut size correct in this case :/ |
| for (size_t i = 0; i < texture_size/2; i++) | ||
| { | ||
| min = std::min<u32>(min, ((u16*)texture)[i] & 0x3fff); | ||
| max = std::max<u32>(max, ((u16*)texture)[i] & 0x3fff); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
degasus
changed the title
degasus
force-pushed
the
custom_texture
branch
from
1c33209
to
dfa66b9
Compare
|
@BigheadSMZ Updated with lots of changes. Now all formats did change, but it automatically convert them on the fly. So please make a copy of your texture packs. Does everything still works fine? |
degasus
force-pushed
the
custom_texture
branch
from
dfa66b9
to
907e5c7
Compare
|
I don't know how the coding side of this looks, but from a users standpoint this is beyond my imagining. No more country code, the original dimensions in the texture, and the hashes seems longer making less chance for two textures to share a name. And the converter, wow. You really outdid yourself. I never imagined any of this when hoping to just see an end to duplicate textures. The original dimensions in the texture is so amazing, something so small but so incredibly convenient! I have tried to replicate any bugs with the old method of dumping and have not been able to. No matter what options I tick with any accuracy in any order, a texture name is always dumped with the same name. Before I could get a total of 6 different possibilities per texture! Words can't properly express my gratitude for all of this, it feels almost too good and perfect to be true. I have not tested other texture packs yet, but for Xenoblade it's perfect. The only thing I can think of that remains is allowing more levels of mipmaps, unless that is already done. And a question regarding mipmaps, is how small should the smallest mipmap be, like 8x8, or 4x4, or even all the way down to 1x1? I think once objects are that small the visual difference between 4 and 8 is almost imperceptible, but I should know what Dolphin expects for a minimum. |
|
Ok two things, one is issue with converter and other is something i remembered. I confirmed what StripTheSoul said here: https://forums.dolphin-emu.org/Thread-hd-retexture-xenoblade-chronicles-hd-texture-pack-v5-7-december-27-2014?pid=353730#pid353730 ... and it seems the converter misses the textures for Sonic Colors (SNCE8P). I redumped a few with Master branch Dolphin and they are indeed dumped correctly ("Fast" accuracy with "Load" checked). This is a very small pack, so redumping this wouldn't be much issue.. but the fact it is missing the textures for this game might mean it affects others. Another thing I noticed Dolphin has always done is dump frames from movie sequences as textures. This happens in Xenoblade and Sonic Colors and I imagine any game with FMVs. It's not a huge issue if it has no work around, but it can be annoying because some videos in Xenoblade dump like 500 (maybe more?) frames. http://i.imgur.com/X2HI2gl.jpg Oh and one last thing I could think of, maybe some identifier in the texture name to know if it has mipmaps or not if this is possible? By looking at a custom texture with no mipmaps included, it is hard to tell if it should have them or not. |
|
Using more levels per texture isn't done right now, but it won't affect this PR. So it should be fine to just create all levels right now and it will be used soon. I'd suggest to go down to 1x1. It's not for a visual difference, it's to remove aliasing. I'm sorry for videos. Technically, they are just textures. So I doubt a hack is worth to detect this kind of textures. Maybe you want to change the video? Just create a custom texture per frame :D I'll add a flag if a texture has mipmaps. Sounds good. |
|
I fell asleep way too early last night...
So the only outstanding issues I can see is the mystery of the converter ignoring Sonic Colors, and whatever is left with mipmaps. |
|
@BigheadSMZ You can check if the Sonic issue is the same as the mipmap issue if the textures without any "_mip%d" parts are converted but the _mapX textures did stay as they are. |
|
I tested packs for: Kirby Air Ride, Legend of Zelda: Wind Waker, Metal Gear Solid: Twin Snakes, New Super Mario Bros. Wii, Paper Mario: TTYD, Resident Evil 3, Star Wars Rogue Leader: Rogue Squadron II, Super Mario Sunshine, Super Paper Mario, Super Smash Bros. Brawl, Tales of Symphonia, and Xenoblade Chronicles. All these packs are working as intended and the textures get converted. Sonic Colors is just being stubborn for some reason. Most of the textures in that pack are not ones that use mipmaps so I don't know about this game... It's only 22 textures total so it's not a big issue as all other packs seem to convert just fine. And one last thing I can think of, if mipmap textures have a flag, is it still worth dumping mipmaps (mip1, mip2, etc)... As long as we know it needs mips and know the min dimensions should be 1, there is not much need to have the dumped mips. |
|
Some games use mipmaps to achieve effects like distance attenuation, so they can be different from a normal mipmap. |
degasus
force-pushed
the
custom_texture
branch
from
907e5c7
to
62462c0
Compare
|
"m" is now in the filename for mipmap textures. @BigheadSMZ I don't think only dumping the first mipmap is worth. Dumping isn't only for custom texture, it's also for debugging. How shall we choose if textures should be converted? Is just an ini fine or do you want me to create a gui checkbox? Still to be discussed: Which hash algorithm shall we use? At the moment we can switch for free. Shall we also mark this texture in some way which let us detect which format it has? Maybe we're able to change it easier in the future. |
|
the hash algorithm is a difficult issue, hardware crc is really fast on intel cpus but gives more collisions than murmurhash3, mumurhash3 is faster than crc on anything older than haswell and on every amd cpu, xxhash shares the same properties of murmur3 and is the fastest, at least on amd and pre-haswell, for new intel’s really don't have the opportunity to compare. |
|
The new CRC is definitely faster on Ivy Bridge and Sandy Bridge, according to my benchmarking... unless the previous usage wasn't murmurhash3? I'd need to have a build with all three options to know for sure. |
|
Mipmaps seems to work fine, also Sonic seems to work fine here, but I have no clue what I've fixed. |
degasus
force-pushed
the
custom_texture
branch
from
62462c0
to
ba96024
Compare
|
Updated with a gfx ini option: "ConvertHiresTextures" I personally have no clue about hashes, so just tell me which one should be used :D Ready for review? |
degasus
changed the title
|
Tested several texture packs now and everything seems to working. If you are still open to suggestions, I have only two at this point.
tex1_256x164_00000000cd3c5907_3 Both of these suggestions are not important at all and I'm happy it works so well and would be content forever in its current state. Thank you for all your hard work, Dolphin and its team never fail to amaze me month after month... |
|
Oh damn, this zeros are a major issue. On linux, they're dumped with 16 digits - but easy to fix: PRIx64... |
|
Both issues should be fixed, build is updated. Are the hashes now 16 digits long? |
|
Yes this format seems perfect now, except maybe typo for underscore in mipmap? most: tex1_36x36_4653193aaa14d048_0.png Honestly it looks better that way except the double underscore so maybe this a bonus. Aside from that, I got nothing left at this point, I don't think it can be made much better than this. :) |
|
yeah, it was a typo. It's hard to test such changes with only a webbrowser and the github edit interface... |
|
Tested it out again with the mipmap marker fix, and everything is beautiful. A tear of happiness ran down my cheek. I can't find any more bugs and the converter is working flawlessly with every situation I throw at it. Once again, even if you get sick of hearing it... amazing and thank you! I would like to make a post in the custom texture section of the forums after this gets merged to master to notify other texture pack authors, unless someone else has this planned. Because I don't understand everything at the level of everyone here, I would appreciate it if someone can verify everything is correct, I'm not missing anything, or if there are better words for what I'm trying to say. I have a rough draft here: http://pastebin.com/FcP3Rsf7 |
|
Back in 2013, @lioncash implemented it as a replacement for murmur, and we concluded that it didn't make much of a difference. On my Phenom II X4 905e it was mostly either equal or xxhash slightly slower; with very few instances where it was faster (by a margin that could might aswell just be within the statistical error range of the measurements). |
|
Murmur is the winner then :) |
|
On my Haswell G3258 I can only recommend using xxhash over Murmur |
|
So xxhash64 sounds like the fastest one. I'll have to wait for PR #1898 |
degasus
force-pushed
the
custom_texture
branch
2 times, most recently
from
8865d1c
to
1d0557a
Compare
degasus
force-pushed
the
custom_texture
branch
from
c2d274f
to
c8d2d85
Compare
degasus
force-pushed
the
custom_texture
branch
from
c8d2d85
to
7cf4dd6
Compare
|
So I think I'm done, new xxhash is in use. Last call for final review and testing |
|
Out of curiosity. Is it safe to use the latest build in this PR to start converting textures before it gets merged to master so we can get a jump start on it? (...as in, hashes are not going to change again) Edit: I know you said not to use edits, but this isn't that important and you can just get it when you get it. I'm gonna give this one more batch of testing now for the next few hours and let you know if I find anything wrong. |
|
@BigheadSMZ I don't know why the name should change again, so I think it's fine to start to redump. |
|
@BigheadSMZ Still dumping? |
|
A couple questions: |
|
I dumped and renamed for about 6 hours straight, and I must say I never get tired of seeing everything textured all the time! Everything seems to work perfectly, I probably have at least 80% of the textures converted, all of them would probably take a full playthrough. I do have a few questions that all tie into the same answer. Can the two formats work together? Like if I nailed all the paletted textures, could I still include other textures that were previously dumped from fast? Or should I wait until I have everything renamed to release the pack? I'm patient enough to do a full playthrough if I must, but if I can take a shortcut to offer people something quick in the meantime then I'll go that route. |
|
@ZephyrSurfer The generated code would be the same, so we should just care about the maintainance. And the collisions should be better because of u64 hash, but xxhash is about as good as murmur3 here. @BigheadSMZ Of course, both formats will live together for a while. But as long as both formats are used, we have to hash the texture in both ways. Maybe with a bit more overhead. I think the next stable release must support the old texture format, but I want to remove it quite after the next stable release. I want to merge those max/min calculations of paletted textures into videocommon to reduces redundant uploads. This maybe fixes the metroid prime stuttering :D |
|
That sounds perfect to me! Both me and frozenwings on the forum are starting a playthrough, between both of us we should hopefully hit everything along the way so this backwards compatibility will be awesome until we finish, and hopefully eliminate that massive 2.8GB pack we currently have as soon as possible. All of this is irrelevant to this PR, so in regards to it, there are no issues I could find and everything is working, including renaming mipmaps and deleting duplicates. I can't think of anything else, so from my perspective it looks good to me. |
|
I think there is a small error in the generation of the old hash, you always executes the xor betwen tex and tlut hash but in the old code the xor was only executed when the format needed it. |
|
@Tinob This is a xor with zero, so it should be fine. Else no non-paletted texture could be loaded. |
|
:) sorry i was thinking in nxor |
Warning: This will break the hashes for most paletted textures.