ColeDeanShepherd / TESUnity

World viewers for Elder Scrolls games in the Unity game engine.
MIT License
92 stars 19 forks source link

Massive choke on CPU time for a single frame. #26

Closed Sinuousity closed 8 years ago

Sinuousity commented 8 years ago

Freeze caused by land tile instantiation and texture decoding. UnityBinaryReader also provides 23 ms delay ( alone this brings the frame rate to 45 ). Profiler overview for reference. I'm not assuming you are unaware of this issue, rather I'm provided more reference material as it is something that needs to be solved at some point. I am still looking into it.

streamingchoke

ColeDeanShepherd commented 8 years ago

Yeah, right now terrains are instantiated, and all dependent textures are loaded, in one frame. This can be solved by either making InstantiateLAND a coroutine that does only one operation at a time (a texture load or a terrain instantiation) or by loading all of the textures asynchronously in the thread pool. The former solution is much easier to implement, but not as effective. I can make a small commit for it in a bit.

Sinuousity commented 8 years ago

Is it necessary to convert the DXT1 dds texture data into the ARGB32 format? Could the project instead use Texture2D.LoadRawTextureData() and keep the texture format as DXT1? At a glance, Unity does seem to support it

ColeDeanShepherd commented 8 years ago

Unfortunately, some Morrowind textures do not have the number of mipmap levels that Unity expects with LoadRawTextureData. Unity expects mipmaps all the way down to a 1x1 texture. I tried to see if I could set the number of mipmap levels in a Texture2D, but mipmapCount is calculated automatically from the texture's width and height, and you can't set it. If you try to use LoadRawTextureData with less mipmap levels it gives you an error. So what I did to get around this was convert the DDS data to ARGB and repeatedly downscale it until all possible mipmap levels are generated.

ColeDeanShepherd commented 8 years ago

Alright, I made a commit that spreads out the texture loads over multiple frames. This should help a bit, but I eventually want to load the textures in the thread pool. I know how I'm going to implement it, but I don't want to right now because I'm trying to take a break from this project for a bit. Let me know if performance still sucks.

Sinuousity commented 8 years ago

No offense, but after doing a little web searching I think you've made some oversights on the dds textures thing! I'm testing further but so far I've been able to load mipless DXT1 textures straight from Morrowind's directory! The post I found works perfectly as of yet

ColeDeanShepherd commented 8 years ago

The issue is with DDS textures that have some of the mipmap levels. If you try to use LoadRawTextureData it will give you an error saying you didn't supply all of the mipmap levels. Some of Morrowind's textures only have mipmap levels that go down in size to like 4x4, but Unity expects all possible mipmap levels. I'll try to find an example texture for you.

Sinuousity commented 8 years ago

Hmm I'm loading any texture I have picked just fine. Textures with alpha channels don't load correctly, probably because they are supposed to be in DXT5?

Oh. And also all textures load upside down.

But if we can get loading DDS textures working and it's faster than converting, I think it'd be worth it just to load them all upside down and I'll flip the V coord in the shader...

ColeDeanShepherd commented 8 years ago

The post you linked to passes false for the mipmap parameter in the Texture2D constructor, so Unity ignores the mipmap data. Try passing true in instead and load "textures\tx_ash_grass_01.dds" and you should get the error I was talking about. The upside-down textures problem is another reason I decode to ARGB, and I flip the textures in DDSReader.LoadDDSTexture

EDIT: The error says "UnityException: LoadRawTextureData: not enough data provided (will result in overread)."

ColeDeanShepherd commented 8 years ago

Also, some of Morrowind's textures use DXT3 format, which isn't supported by Unity. I hope I'm not coming across as defensive. I tried using your approach initially but Morrowind's textures hit some corner cases that LoadRawTextureData can't handle, so I ended up converting them to ARGB and downscaling/flipping them to get around those corner cases.

Sinuousity commented 8 years ago

Not at all! But from what I gathered from the profiler, we really only need to compensate for DXT1 textures, as those seem to be the issue. I'm looking to throw away all your hard work.

ColeDeanShepherd commented 8 years ago

Well, try out my latest commit if you haven't already, and let me know if that helps with the freezes. When I move the texture loads for terrains into the thread pool the freezes should be gone, for the most part. Unfortunately, there's a really slow internal Unity function called Terrain.GenerateBaseMap which is called when setting terrain splat maps, which can only be done on the main thread, which causes freezes. I can't think of anything to fix this aside from using meshes instead of Unity's terrain system.

ColeDeanShepherd commented 8 years ago

Of course, using the thread pool doesn't speed DXT1 texture loading, but it will move the freezes to a background thread. I've looked into optimizing the DDS loading functions, but I couldn't think of much I could do at a glance. A while ago I implemented pre-allocation of arrays for those functions to avoid garbage collection, but that sacrifices the thread safety of the functions, so I reverted those changes.

Sinuousity commented 8 years ago

I wrote a DDS inspector in a separate project to load and inspect textures. (See images) The structure of my DDSLoader turned out a lot like the one that exists in the project just a bit smaller.

I'm assuming you already know a bunch of this information but I'm covering it just for consistency. It seems like most flags in the DDS files are not used in MW. (The entire PixelFormat is unused?) MipMapCount is used, and we can use that to decide whether or not to use mipmaps on the Texture when we create it.

The only way I have found to automatically pick between DXT1 and DXT5 formats is comparing the size of the texture data to the size it would be in each format. This works well, though and it seems to handle DXT3 files perfectly as well!

dxt1loaded dxt3loaded dxt5loaded

The textures are still flipped vertically for the time being, and the alpha channels of DXT5 loaded textures seems incorrect.

ColeDeanShepherd commented 8 years ago

Certain flags in DDS files are only used in some cases, depending on the texture format. You can find information on the DDS file format here, here, and here. To differentiate between DXT1, DXT3, and DXT5 I use DDS_PIXELFORMAT.dwFlags and DDS_PIXELFORMAT.dwFourCC, and to determine if textures have mipmaps, I'm using DDS_HEADER.dwCaps. As to the alpha channel being wrong, you have to consider each row of alpha indices as a little-endian 3-byte unsigned integer.

Nice job. Is this utility a personal project, separate from TESUnity?

Sinuousity commented 8 years ago

It is a separate utility currently but it wouldn't take much to implement it over. I'm actually thinking about incorporating a bunch of custom Editor Windows for the project as I know it can help a lot for efficiency. Also as it stands I always feel intrusive when I change too much of the project, so focusing on adding that new layer of functionality would be a bit relieving.

Sinuousity commented 8 years ago

Bunch of improvements to the DDS Inspector. -Lots of behind the scenes stuff -File auto-reloading (except DDSFile data is lost when the script is recompiled & needs to trigger reload) -New texture preview is self contained - should be able to use the same method for a model viewer -Invalid file loading error catching - no errors or warnings in Log when loading fails -Fancy colors and buttons -All window contents behave very nicely now when resizing window.

I'll be porting this to TESUnity tomorrow and I'll finish cleaning it up there. I'll also be writing a BSA file browser window since most files aren't loose and won't be loaded directly from the install directory. The BSA browser will replace Window's file browser (or maybe I'll keep both and user can choose where they want to load from)

ddsinspector2

ColeDeanShepherd commented 8 years ago

Nice job man. Feel free to keep it a separate project that you own that TESUnity just uses if you make it open source.