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

Allow delayed loading of texture replacements #15025

Merged
merged 12 commits into from
Oct 25, 2021

Conversation

unknownbrackets
Copy link
Collaborator

Delayed loading (pop in) is not really ideal, especially if replacements aren't just higher detail but have some differences (I think for more simplistic games, they can even be used to translate a game.) But hitching is really annoying, so arguably the replaced images "hitching" in without affecting gameplay is also bad, but less bad.

This takes a very simple step and uses a frame budget to decode texture level data into RAM but avoid significant hitching. It keeps this decoded data cached for a while, so it can skip decoding if the texture is needed again in a couple minutes.

It might be nice for this to be configurable (both whether you would allow popping, and maybe some way to balance RAM usage), but wanted to open this to allow people to experiment and see how this impacts hitching.

This doesn't move texture decoding to threads, but that wouldn't be much harder to do. A more advanced technique (loading texture in thread until queue runner, and dynamically selecting texture and maybe shader at that point) is somewhat complex, but ultimately would probably result in some hitching or pop-in too, so this is an experiment to see how bad pop-in is.

Feedback on builds under Artifacts on the Checks tab appreciated.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

Cool idea, fails to load replacements for me:

09:52:896 drawlogo thr E[G3D]: Core\TextureReplacer.cpp:839 Could not load texture replacement: R:/PPSSPP/memstick/PSP/TEXTURES/ULJS00321/pngs/00000000c6223a486d2b285b.png - png_image_finish_read: invalid argument
09:56:932 drawlogo thr E[G3D]: Core\TextureReplacer.cpp:839 Could not load texture replacement: R:/PPSSPP/memstick/PSP/TEXTURES/ULJS00321/pngs/000000000b76171cb4a6bef0.png - png_image_finish_read: invalid argument
10:00:935 drawlogo thr E[G3D]: Core\TextureReplacer.cpp:839 Could not load texture replacement: R:/PPSSPP/memstick/PSP/TEXTURES/ULJS00321/pngs/00000000133d1876f794aa32.png - png_image_finish_read: invalid argument
10:04:938 drawlogo thr E[G3D]: Core\TextureReplacer.cpp:839 Could not load texture replacement: R:/PPSSPP/memstick/PSP/TEXTURES/ULJS00321/pngs/0000000005ce30543ef4bcbf.png - png_image_finish_read: invalid argument

anything special that needs to be done for it? Used vulkan and just tried to run my stuttery test texture pack I have on ramdisk.

@unknownbrackets
Copy link
Collaborator Author

Blarg. I got myself worried about the width changing and crashing from out of bounds and forgot to validate PNGs after changing, I seem to be forgetting to retest after minor changes more often these days... I thought I did. Darn it. Anyhow, fixed, just needed the png.width * 4. Sorry.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

It replaces the textures nicely on boot, but then crashes when the video should start(movie is not replaced there) actually it's just another load screen that must be different in some way, it also crashes when loading a savestate with texture replacement active or when activating a texture replacement in-game after loading it without.
assertion fail

>	PPSSPPDebug64.exe!_invoke_watson(const wchar_t * expression, const wchar_t * function_name, const wchar_t * file_name, unsigned int line_number, unsigned __int64 reserved) Line 224	C++
 	PPSSPPDebug64.exe!_invalid_parameter(const wchar_t * expression, const wchar_t * function_name, const wchar_t * file_name, unsigned int line_number, unsigned __int64 reserved) Line 113	C++
 	[External Code]	
 	PPSSPPDebug64.exe!ReplacedTexture::Load(int level, void * out, int rowPitch) Line 869	C++
 	PPSSPPDebug64.exe!TextureCacheD3D11::LoadTextureLevel(TexCacheEntry & entry, ReplacedTexture & replaced, int level, int maxLevel, int scaleFactor, DXGI_FORMAT dstFmt) Line 693	C++
 	PPSSPPDebug64.exe!TextureCacheD3D11::BuildTexture(TexCacheEntry * const entry) Line 543	C++
 	PPSSPPDebug64.exe!TextureCacheCommon::ApplyTexture() Line 1679	C++
 	PPSSPPDebug64.exe!DrawEngineD3D11::ApplyDrawStateLate(bool applyStencilRef, unsigned char stencilRef) Line 434	C++
 	PPSSPPDebug64.exe!DrawEngineD3D11::DoFlush() Line 606	C++
 	PPSSPPDebug64.exe!DrawEngineD3D11::Flush() Line 142	C++
 	PPSSPPDebug64.exe!DrawEngineD3D11::DispatchFlush() Line 150	C++
 	PPSSPPDebug64.exe!GPUCommon::FastRunLoop(DisplayList & list) Line 1062	C++
 	PPSSPPDebug64.exe!GPUCommon::InterpretList(DisplayList & list) Line 1005	C++
 	PPSSPPDebug64.exe!GPUCommon::ProcessDLQueue() Line 1206	C++
 	PPSSPPDebug64.exe!GPUCommon::UpdateStall(int listid, unsigned int newstall) Line 837	C++
 	PPSSPPDebug64.exe!sceGeListUpdateStallAddr(unsigned int displayListID, unsigned int stallAddress) Line 381	C++
 	PPSSPPDebug64.exe!WrapI_UU<&sceGeListUpdateStallAddr>() Line 200	C++
 	PPSSPPDebug64.exe!CallSyscallWithoutFlags(const HLEFunction * info) Line 652	C++
 	[External Code]	

@unknownbrackets
Copy link
Collaborator Author

Ah, sorry, D3D11 was still trying to use the texture. Cleaned that up and centralized a bit.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

Thanks:),
unfortunately it still doesn't solve the stutter completely, when a dialog loads asking about savedata I can see the font pop in, but instead of one freeze on the start it kind of has multiply freezes now for each frame(the dialog is animate and stretches out). Same thing for other textures, they do pop in, but there's still a freeze despite running everything of ramdisk.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

https://youtu.be/58avCsikNqo a short clip showing how it pops while still stuttering.

@unknownbrackets
Copy link
Collaborator Author

If any individual load takes more than a frame, it'll currently still hitch (would be solved using a thread), but the multiple freezes should be smaller than the original freeze would've been. That seemed pretty stuttery though, seems worse than I'd expect the sum... maybe I made a mistake somewhere...

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

That stuttery screen with dialog animation is actually much worse than without this feature, kind of looks like it invalidates the texture each frame as without the feature it just quickly loads and disappears without any delays.

edit:
old behaviour
https://youtu.be/4PxA7h0C7GU

I guess the black screen definitely is shorter with this feature, so it's where all the loading happens, I had to check, but this small dialog actually loads 16 textures, 11 of which are 512x512 font textures.;o

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 17, 2021

Combined ZIM + pop in is enought to have it practically stutter free:).
https://youtu.be/b5Y7WUPlW9Q

@unknownbrackets
Copy link
Collaborator Author

So sounds like it'd be useful for me to add the threaded decoding here.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 21, 2021

If I understand correctly, threaded decoding would make it no longer require to fit within frame budget, so assuming no other bottlenecks, would make it silky smooth and just make it pop in later depending on hardware performance?

If soo that'll be great, definitely a better choice for gameplay than freezes/stutter through an option to disable delay might still be nice to have for people that doesn't care about gameplay experience and just want to record video with built-in recorder.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 22, 2021

I think there's some kind of issue with the new commit, didn't notice much of a change so I restarted the game(via ctrl+B) and after a few seconds, before even reaching the stuttery part it simply hang, it didn't crashed or anything, the window is still active and not frozen either, it just stays there on black screen forever and while it logs input, it doesn't react to it, so can't even leave to menu. Edit: now it hangs like that every time the moment it hits the stuttery moment, even through I did close it through task manager completely.

@unknownbrackets unknownbrackets marked this pull request as draft October 23, 2021 05:47
@unknownbrackets
Copy link
Collaborator Author

I probably need to test with a more varied texture pack...

If you comment out g_threadManager.EnqueueTask(new ReplacedTextureTask(*this, threadWaitable_), TaskType::IO_BLOCKING);, does it still happen? That will make it never actually load the replaced textures, and it'd build up trying to load them. If it still hangs, there must be something else...

If that does prevent the hang, what about making bool ReplacedTexture::Load(int level, void *out, int rowPitch) { return false straight away? If that alone prevents the hang, it probably just means some form of problem with actually applying the texture (you'll get corrupt textures though with that change.)

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 23, 2021

Both prevent the hang, however the first one prevents it completely, the latter despite glitches still makes it hang when restarting the game. Edit: maybe I should word that differently, as in the latter doesn't prevent the hang, just slightly changes how the hang can be reproduced. Either way the key to hang with and without this test change is restarting the game as if something wasn't cleared/restarted properly.

The only differences is it will work once after re-running PPSSPP, while previously it was hanging every time after first game restart even when PPSSPP was fully closed and reopened and also previously it continued to hang after few boot screens, with the last change it crashes instantly when restarting the game via ctrl+B.

If the texture is being loaded and we stop or reset, make sure it stops to
avoid any crash or hang.
@unknownbrackets
Copy link
Collaborator Author

Hm, I'm not reproducing that currently. I did find things would go badly if you reset emulation while a texture was still busy loading, though. Should be improved now.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 23, 2021

Unfortunately still starts hanging after first game restart, then it continues to hang during first run as if something in the system would continue affecting it until I restart windows even if I manually close all PPSSPP instances through task manager.

I don't use any 3rd party overlay software nor driver hacks, it's pretty bare bone win 10, chrome and Visual Studio running, tries to change backends and settings as well to see if anything affects it, but doesn't seem like it's the case.

One weird thing just happened, I ran PPSSPP which was already hanging through DebugLog.bat and it somehow passed the hang moment and continued to replace textures further, until start menu. Maybe --log affected some timing, because every single time it hanged at earlier spot in previous tries.:o I was writing that it possibly avoided the issue completely then I got back into the game and tried to continue and it still hanged. Not sure if this helps, but here's the log. The PPSSPP doesn't crash during the hang, but stops logging doing anything except input, so the hang had to start near the end of this log.
ppsspplog.zip
Also made another where I load a savestate and tried to go through menus/screens fast until it hanged(much smaller file).
hang shortly after savestate load ppsspplog.zip

@unknownbrackets
Copy link
Collaborator Author

Still haven't reproduced the problems, I'm mainly testing using Persona 3 Portable and its texture pack that was posted on the forum - tried multiple backends, at 4x. That said, I realized maybe running a parallel loop from a task could cause problems, maybe that's the issue?

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 24, 2021

Yup, that appear to fix the problem:).

@unknownbrackets unknownbrackets marked this pull request as ready for review October 24, 2021 04:02
@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 24, 2021

The texture pop-in at least in case of AI-upscaled textures which are faithful to original isn't that noticeable and PNG texture pack works just as smoothly as the ZIM one with this maybe with just slightly longer texture load:).

Unfortunately both formats still suffer from ocassional stutter, not sure what's causing it, I get it doesn't matter if I have the texture pack on hdd, ssd or ramdisk, not sure what bottleneck causes that.
Through this is a texture pack made in very lazy way of (c)upscaling whole folder with everything dumped, generating file list and placing it inside two separate textures.ini for png/zim made purely for testing so it's probably worst case scenario out there.

@unknownbrackets
Copy link
Collaborator Author

Hm, I wanted to let there be some time to avoid pop in for faster loading textures. There's two possibilities for the stuttering:

  1. It's not the decoding anymore, but the upload to VRAM, that is causing the stutter. In this case, we'd really need to create the actual texture entirely on the thread, which would be more work but is in theory doable.

  2. The leniency to allow some delay plus the upload time is adding up, causing the lags.

We could validate if it's 2 pretty simply, by changing constexpr double MAX_BUDGET_PER_TEX = 0.25 / 60.0; to constexpr double MAX_BUDGET_PER_TEX = -1.0;. This would make it basically always pop in a texture later, theoretically minimizing lag. It might be best, and maybe I'm worrying overly about avoiding pop in...

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 24, 2021

Setting it to -1.0 just stopped replacing textures, I tried to instead divide it again by 10000.0, this felt less noticeable in-game, but maybe it's just a placebo as ocassionally it still stuttered.

@unknownbrackets
Copy link
Collaborator Author

Ah, sorry, I fixed it to allow 0.0 which should have the purpose I intended.

Watching the log with the pack, I do sometimes still drop a frame here and there, which I do think is from the time uploading the texture. Still, it does seem like much reduced lag.

-[Unknown]

@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 24, 2021

Yeah even with 0 it still has it's moments, still even if not perfect, it's a huge improvement.(Edit: by huge improvement I mean this PR, not the 0 change ~ still not sure if it really has an impact)

Even with this it's worth converting textures to ZIM format as the stutter while in same moments is a bit shorter with ZIM's(I mostly get it by sound by now as there's no freezes whatsoever with this PR).

@hrydgard
Copy link
Owner

I think this looks fine overall, was slightly afraid that it would interfere with the texture loading rework that's slowly starting to take shape in my head, but it doesn't.

I've been swamped lately so haven't had time to really test this, but I'm happy to get this in, and if needed we can improve it further later.

@hrydgard hrydgard added this to the v1.13.0 milestone Oct 24, 2021
@hrydgard hrydgard added GE emulation Backend-independent GPU issues User Interface PPSSPP's own user interface / UX labels Oct 24, 2021
@LunaMoo
Copy link
Collaborator

LunaMoo commented Oct 25, 2021

Made a short video that includes gameplay to show the delay with this PR as it is now, but with ZIM textures it's hard to notice, max few frames in the heaviest places like savedata dialog after boot screens(11 font 512x512 textures + 5 smaller ones all upscaled x4) that otherwise would just freeze and load with a delay:
https://www.youtube.com/watch?v=nqezLidcP6E

The water in-game is glitchy, but that's cause missing mipmap interpolation, has nothing to do with the feature.

@Saramagrean
Copy link
Contributor

Wow, it fixed stuttering issue I encountered in my texture pack. here comparison video.
https://youtu.be/_XhTzQdup2o

@hrydgard
Copy link
Owner

hrydgard commented Oct 25, 2021

Let's get it in.

@superostrich
Copy link

superostrich commented Jul 28, 2022

This has completely fixed the stuttering in the games I've made texture packs for!

The only downside is that re-loading from a save state or toggling texture replacement off and on no longer causes the textures to be re-loaded from disk, which is an important feature for authoring textures, since it lets you rapidly iterate and test in-game.

It would be useful to be able to disable this feature, or flush the texture cache and re-load replaced textures from disk if the user toggles texture replacement off and on,

@unknownbrackets
Copy link
Collaborator Author

You can disable this using ReplaceTexturesAllowLate = False in the ini. I suppose it might cache that the file doesn't exist? Probably we can improve this, I think it might be the later pull that reduces IO if anything.

-[Unknown]

@superostrich
Copy link

Just tested; setting ReplaceTexturesAllowLate = False does not cause the textures to re-load from disk when toggling texture replacement, or loading from a save state.

A hotkey to reload replaced textures from disk, would be very useful!

@unknownbrackets
Copy link
Collaborator Author

Please check #15740 when it's done building (Artifacts on the Checks tab.) This should make disabling and re-enabling clear cache and reload them.

-[Unknown]

@superostrich
Copy link

This should make disabling and re-enabling clear cache and reload them.

Works perfectly, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues User Interface PPSSPP's own user interface / UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants