Using the Texture right-click menu, the Editor eventually trips on the `TileFreeList.Num() == 0` assert in `FVTUploadTileAllocator::FStagingBuffer::Init()` shortly after conversion completes. The staging buffer’s backing memory is freed but free tile indices still got returned later.
After some tracking down, the cause appears to be bit truncation on the `FHandle::StagingBufferIndex` during calls to `FVTUploadTileAllocator::Allocate()`. This probably only happens during this conversion process in the Editor, because these systems get slammed with an unusual volume of work. Later when tiles get freed, they index into the wrong staging buffer and add duplicates to the free tile list, which among other invariant breakages causes the premature release of its backing buffer.
Just to work around this, I introduced some additional Editor-only criteria in the `FormatDescs` loop for selecting the format buffer, so we skip and select/allocate a new one when there’s a risk of having more than 256 staging buffers. As a sanity check, I also introduced an assert for all platforms+Editor to make sure indices are in-range just before constructing the handle, and so far no trips.
Our project’s textures in-repo are non-SVT. Converting a large set of textures (such as all textures in the project), which pull in a large set of referencing Materials, eventually trips an assert in the Editor in `FVTUploadTileAllocator::FStagingBuffer::Init()` shortly after conversion completes. You may be able to hit the issue performing this on a UE5 example project with equivalent volume of unconverted textures and dependent materials.
Hi Garrah, thanks for bringing this to our attention. Could you open a pull request for your changes to the Virtual Texture buffer upload logic? I have not heard that this has been an issue for us, but hitting this assert seems to occur only under specific circumstances, so it may be a matter of time before we get more reports about this. Therefore, it would be worth evaluating whether we want to integrate your new safeguards into the engine. Let me know what you think!
No worries about that. We have not encountered this particular limitation of the virtual texture upload path, so it would be good to have your proposed changes in a small patch that we could run against some of our projects to evaluate the impact. Otherwise, the only advice I could give you is that you can stick with your change, but you would need to be careful about the memory consumption. Let me know if that makes sense.
In short, while the assert is fairly straightforward, one downside of the (fairly simple) skip logic I introduced is that it effectively ‘locks-off’ the ability to allocate from that format buffer, causing it to be skipped despite potentially having tiles freed in its earlier staging buffers, until tiles from the very last (N-1) staging buffer begin to free. This has the potential to force (many?) more allocations than necessary. At face, maybe that’s still acceptable, but owners of this engine code may want a more ‘wholistic’ solution. Curious on your thoughts.