Replacement: Move IO checks to saving thread #15489
Conversation
This might be used for fonts, for example. They could be replaced, so no reason not to export.
This will spin up more threads that might not actually save, but it will remember this in savedCache.
This does mean an actively saved texture won't recheck, but it's close to the old behavior where you could delete a new texture and it'd soon be resaved. This was convenient sometimes.
For clarity, it's required.
|
I will see separately if I can still reproduce #15488. I might have ended up both breaking it and fixing it again while working on #15487, so it might not be a real issue. Moving the checks to the tasks should be good, yes. Seems like a good workaround for the reading-upload-memory issue. I know that this can be an issue in D3D11 too (you commonly get write-combined memory when wrapping buffers for write there) but not sure in practice how many people it will affect or how badly... |
| if (ignoreMipmap_ && level > 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This check is probably what saved us before, removing it resulted in #15492 .
I know what's wrong though, code to detect the fake mipmap situation isn't quite right, fixing.
There was a problem hiding this comment.
It must be the break I removed in BuildTexture, I assumed it was just a mistake. This check is still there, just moved above the cachekey.
-[Unknown]
There was a problem hiding this comment.
Ah right. Well I just had a quick skim of this one after actually finding the real root cause. No fault of this PR.
Didn't check perf on Android, just that saving still works on Windows. For me, under Vulkan, saving worked fine with upscale (didn't reproduce #15488), but maybe that's other backends.
This fixes saving "fake mipmaps" that force a constant texture level to 0, and moves IO onto the thread. It does allow retrying saves every 5s, because before it intentionally repopulated deleted files. Even if this queues up a bunch of tasks, I expect it to be much lighter than before. That said, perhaps CPU_COMPUTE is a lie moreso now...
Further, on Vulkan, this moves decoding to a temporary buffer when saving texture data. For simplicity, this buffer matches the target decode aligned stride, and maybe could
std::move()to the thread as a later optimization (I'm sure the IO and PNG encode are a more significant part of performance, though.)This also disables the save checkbox if not replacing, which was already how it actually worked.
I didn't look at Direct3D 11, but it could probably have something similar done to avoid reading from the upload buffer. I think it's useful to save the upscaled data (at least when CPU upscaling), so I'd rather we not re-decode. That sounds messy and wasteful for performance, anyway.
Likely to help #15478.
-[Unknown]