Parser and texture improvement - #1381
Conversation
|
Basically, a lot of this is code that hasn't been touched since things were written originally, and consequently it's not got the benefits of stuff like HashSets :) (remember the code dates from .Net 1.5) A lot of the bug comments in the code may longer be valid- getting multi-threading going safely was pretty painful. |
|
It's 3:00 AM right now in Japan, so I'd like to build? it and give it a try tomorrow! |
|
Further thoughts: Only other thought is that fundamentally, most of our users aren't route builders, and whilst chasing reload times etc. here is never bad, it's probably secondary. |
|
I built and ran the program. log.txt RouteViewer failed to launch and displayed the following message. |
|
@ginga81 can you test again? and also test the latest master too? if the fix in this PR works i will take it as separate PR for master bracnh. |
|
I downloaded and built it again, but neither RouteViewer nor OpenBVE would launch—just like before. |
|
https://github.com/adfriz/OpenBVE/tree/parser-and-texture-improvement |
|
thats odd, idk why it doesn't work on you... |
|
debug vs. release? |
Ever since it was pointed out that the build wasn't set to "Release," I have always used this setting.
|
net 1.5 ? is this the time when openbve still using Tao framework? if im not mistaken.
is this going to be a block parser like the csv b3d parser? well, the motivation of this PR was the loading/reloading time is too much, although it may depends on how the route made by the route developer... |
hmm release or debug is not the problem i think, i always test with release build for the more accurate timer log. |
|
can you try rebuild all ? or maybe using command line ? maybe like this? idk if that command is correct, im not familiar with linux. |
|
As I mentioned earlier, I am building the project using MonoDevelop. I'm not sure what the difference is, though... |
|
this is hard... the PR build checks shows all green... @leezer3 any thoughts? @ginga81 can you try build the master branch? https://github.com/leezer3/OpenBVE.git i wanna know if the master branch has this problem on linux |
It failed to start as well. |
|
I'll see what I can find tomorrow. Not sure why this has changed, maybe altering the .Net version has changed the version of the toolset it's pulling or something.... |
|
#1380 |
|
@ginga81 Leezer3 already merge a fix for that missing opentk dll, if you have time can you try it? |
|
https://github.com/leezer3/OpenBVE |
|
https://github.com/adfriz/OpenBVE/tree/parser-and-texture-improvement |
- Pre-seed the texture cache with the decoded texture (not the handle) at register time, preventing a null reference when transparency is queried. - Add a registered texture lookup table to avoid scanning every registered texture for each new registration. - Preserve unchanged textures on route reload to avoid re-uploading them. - Use HashSet for missing files / failed objects / failed textures lookups. - Replace Thread.Sleep(1) with Thread.Yield in the CSV route parser.
- Measure route parse, post-parse setup, texture registration and texture decoding time, and report them in a summary message after loading.
- Fix TextureOrigin.Equals() inverted logic (all branches were wrong) - Add GetHashCode() to PathOrigin for proper dictionary lookups - Fix ByteArrayOrigin.NumberOfFrames (was byte count instead of 1) - Fix InvertLightness loop bound (MyBytes.Length -> MyBytes[frame].Length) - Fix GrayscaleAlpha stride bug (i+=4 -> i+=2, stride calculation, TexImage2D moved outside loop) - Add thread-safety locks for textureCache modifications - Replace ContainsKey+indexer with TryGetValue in ObjectLibrary.ShowObject - Consolidate 3 lock acquisitions into 2 in RegisterTexture - Skip GC.Collect on route reload (only on full unload) - Add per-material alpha cache in ShowObject to avoid repeated textureCache lookups
- Replace ~20 ElementAt() calls with foreach in ApplyRouteData.cs and RouteData.cs (O(n^2) -> O(n)) - Cache brightness lookup index in GetBrightness() to avoid full blocks scan per call - Replace Regex.Matches per-line with IndexOf loop in Preprocess.cs - Replace .ToLowerInvariant() == with string.Equals(OrdinalIgnoreCase) for 'with' command - Remove duplicate ObjectDictionary initialization
- Add Stopwatch timing around ParseRouteForData and ApplyRouteData in CsvRwRouteParser - Add PluginParseTime and PluginApplyTime fields to HostInterface - Update log format: parser: X ms (parse: X ms, apply: X ms) | textures: X ms (decode: X ms)
Host.RegisterTexture (RouteViewer + main game) and TextureManager.RegisterTexture all checked File.Exists before passing to Host.LoadTexture, which has its own File.Exists check. Removed redundant checks from Host.cs wrappers.
…dificationTime) File.GetLastWriteTime() and FileInfo.Length were called in PathOrigin constructor for every texture registration. These are only needed during route reload (TextureFileUnchanged), so deferring them avoids 2 FS syscalls per texture during initial load.
…erations TextureManager.RegisterTexture had a backwards loop calling Array.Resize for every trailing null entry — O(n) per registration on a large route. Removed entirely; GetNextFreeTexture already handles growth via power-of-2 doubling. Also removed redundant File.Exists check in TextureManager, flattened textureCache pre-seed into single if-condition, and replaced ContainsKey+Add with indexer assignment in ObjectLibrary.ShowObject cache miss path.
98a85a0 to
be9211f
Compare
|
I've reset this branch to the current master branch. If you download it again and try building, it should have the same fix. |
be9211f to
341858b
Compare
|
I'm now able to run RouteViewer as well! |
|
Try reload the route by pressing f5 and show the timing again |
|
Information Route reloaded in 10173 ms | parser: 10055 ms (parse: 8917 ms, apply: 1111 ms) | textures: 0 ms (decode: 0 ms) | setup: 4 ms, visibility: 196 ms. |
|
Nice. Texture = 0 ms means it's not reloading the same texture again. If you want to try, try edit just 1 texture image and reload it again. If decode not 0 ms, it means it should only reload the edited texture. |
|
Information Route loaded in 16839 ms | parser: 16804 ms (parse: 15483 ms, apply: 1225 ms) | textures: 5938 ms (decode: 0 ms) | setup: 7 ms, visibility: 236 ms. |
|
Huh that's odd, it should changed just by reloading by pressing f5 not reopening the route. Mind testing on the demo route ? https://openbve-project.net/documentation_hugo/en/examples/demoroute.html In my test I just edit 1 of the tree image with red scribble. And it should count the decode timer when the route reloaded. Or maybe your PC is too fast it couldn't record the timings. 🤔 |
|
I installed DemoRoute1 from the package and overwrote |
|
編集した画像をルートビューアにドラッグするのではなく、ルートをリロード（再読み込み）するという意味です。 画像をドラッグしても何も起きませんが、それは仕様（想定通りの動作）です。 少なくとも、画像を編集した後にルートをリロードした際、初回のリロードほど時間はかからないですよね？通常のリロードと同じくらい速いはずです。 |
|
Of course, after changing the image, I pressed F5 in RouteViewer to reload it. |
|
The demo route was so fast that the reload time was too quick to even measure—feels around 0.1 seconds. |
|
Oh ok, it seems I misinterpreted 😅 Alright, at least nothing broken when you test it. Thanks for testing this PR 👍 If you found any missing or broken route when trying this PR, just report here. I'll try to make sure it fixed. |
Textures not reloading:The trouble you're seeing is because File.LastWriteTime gets cached by the framework / kernel filesystem abstraction layer. Using a FileSystemInfo and calling the Refresh function before accessing the date-time is marginally more reliable, but nowhere near 100% here on Windows. |
This may or may not the cause? I mean the route viewer still not have auto reload like object viewer, so it's still need manually reloaded if anything changes. But, thanks for the info leezer3, i will take a look. |
f603adc to
341858b
Compare
|
ugh something wrong with my local git. unfortunately i cant delete that unnecessary force push history |
|
Personal opinion... These two stages are distinct; they differ in whether immediate display is required or if a longer loading time is acceptable.
It would be extremely helpful to have a mode that displays only the area within the set draw distance—ahead and behind—as quickly as possible. |
|
@ginga81 is that new commit working on you? |
|
Yes! I think it’s become really fast and convenient! |
thats weird, i didnt edit that options window. if you resize it, will it looks correct now? |
This is how it turned out after resizing.
|
|
are object viewer also like that? |
|
Here is how ObjectViewer looks now. The width adjusts dynamically, though the height remains unchanged. |
3 participants
Heavily WIP.
Mostly about caching and redundant stuff.
maybe im doing this too much.
But some obvious imporvement is the parsing and reloading in route viewer.
From my own test, loading tohoku shinkansen route, the 1fine-Hayabusa1v1.00.csv results is :
For the first load is down from 95s to 25s or less
For reloading is down from 35s to 5s or less.
Note: this results may vary, depends on the system specs and system load.
This commit still has the timer logging code.
That timer log can be accessed in F9 menu on the very bottom rows.
@ginga81 do you want to try this?