Time Nick Message 00:25 Zughy[m] I don't understand why it only affects Android but thank you savilli, it worked 00:36 MTDiscord Because android is the most different setup 00:37 MTDiscord It could affect everything except Android, but you would notice it and do smth sooner ofc 08:24 sfan5 bad param2 means what? 16:23 MTDiscord It means > 5 for wallmounted nodes 16:26 MTDiscord Seems like it would be nice if MT just always defined sane behavior for those things, like "treat any value >5 as if it's 0" or "treat all values as mod 6" or something. At least, better than "crash, but only on certain platforms". 16:26 MTDiscord Is there an Issue for it? 16:29 schwarzwald[m] What if we documented it as undefined behavior so it would be clear values > 5 had no meaning? Is defining the behavior worth it? 16:31 rubenwardy it should not crash if the value >5, no need to define it 16:31 MTDiscord The server shouldn't be able to trigger UB in the client. I'll fix it. 16:32 schwarzwald[m] Oh, that point is hard to dispute. :| 16:32 MTDiscord Does anyone use "mesh cache" feature? It's disabled if smooth lighting is enabled, and smooth lighting is enabled by default. 16:33 rubenwardy btw: by defined/undefined behaviour, we're talking about the Lua API. Not C++ 16:34 rubenwardy the server not sending invalid param2 would fix this for older clients, but it should probably be fixed in the client too 16:35 MTDiscord What does the "mesh cache" feature do? I've got a client that's been getting like 2fps since 5.6 and I've been looking for some kind of config changes that would speed it up; would disabling smooth lighting improve that? 16:38 MTDiscord Well, it "caches" meshes. The engine can either recalculate normals and bounding box every time you see a mesh, or calculate it on loading and store the mesh in the cache. 16:38 MTDiscord The feature looks very old and not popular. I wonder if I should fix it or just remove it. 16:41 MTDiscord Is this just for mesh nodes, or does it speed up entity initialization too? I think my problem seems most strongly correlated with scenes that have a lot of ents. 16:41 MTDiscord Like I guess I don't know whether it's specific to certain kinds of meshes, like mapblock meshes, node meshes, or ent meshes... 16:41 MTDiscord It's only for mesh nodes 16:42 MTDiscord Odd that it would be tied to smooth lighting 16:42 MTDiscord Because you can't cache a mesh with smooth lighting 16:42 MTDiscord You need to change it every time 16:42 MTDiscord I thought you'd need to change it every time with unsmooth lighting too. 16:42 MTDiscord I can try turning off smooth lighting and see if it makes any difference, at least. 16:43 MTDiscord If it does then I might argue that we try to keep the feature, but if not (which seems likely) then yeah, it might not be worth keeping. 16:43 MTDiscord You also need to enable mesh cache in advanced settings 16:43 rubenwardy test the performance improvement, and remove if it doesn't improve performance and does cause issues 16:43 rubenwardy it's disabled by default? 16:44 MTDiscord Yes 16:44 rubenwardy sounds like a removal candidate :) 16:44 MTDiscord Remove it if it doesn't improve anything, regardless of whether it causes issues. Existing is itself at least a minor issue, and a feature needs to justify its existence to counterbalance that. 16:44 rubenwardy true 16:45 rubenwardy we could do with a suite of benchmarks 16:45 MTDiscord I feel I have too high-end hardware to test that. Maybe it does for someone. 16:45 MTDiscord lemme check the settings involved and try configuring my currently-pathologically-performing client and see what happens... 16:45 rubenwardy the benchmark suite could then run on people's computers, and we could have CI set up for it running on different hardware 16:45 rubenwardy https://www.minetest.net/benchmarks/dev/bench/ extended 16:46 MTDiscord If you want low-end hardware, just run my minetestcast client, which wraps an MT client, ffmpeg and Xvfb in a docker container for automated livestreaming. Basically no 3D acceleration support, and the sandboxing seems not so nice for the CPU either... 16:46 MTDiscord which settings are involved in this? 16:46 rubenwardy I think our low end hardware target should be the RPI 4 16:46 rubenwardy or an Android phone 16:47 MTDiscord Interesting that smooth lighting is disabled for android by default 16:47 MTDiscord I found smooth_lighting = false and enable_mesh_cache = true ... is that enough to enable it? 16:47 MTDiscord Yup 16:48 rubenwardy is enable_mesh_cache still disabled on Android though? 16:48 MTDiscord Looks like so 16:48 rubenwardy for a benchmark suite, you probably want low end, mid, high end, and old. And different platforms 16:48 schwarzwald[m] Related to system requirements, are we planning to bump the Irrlicht C++ standard to C++14? I thought we already did this for Minetest proper, or am I forgetting? 16:49 rubenwardy that would be allowed 16:49 rubenwardy as we already require C++14 for Minetest 16:51 MTDiscord When you use C++21 on regular basic, C++14 feels extremely old 16:52 rubenwardy C++17 is when it starts to feel almost like a good language 16:52 rubenwardy almost 16:52 MTDiscord Performance improvements, if present, aren't dramatic enough to be definitely noticeable. I'd say it's well within the massive margin of error of my methodology. 16:52 rubenwardy [DELETE] 16:52 rubenwardy when was it added, and by who? 16:53 rubenwardy !title https://github.com/minetest/minetest/commit/dd4c21c1808acedfbcf8402c09ce9129b6ac31c7 16:53 ShadowBot rubenwardy: Add option to enable mesh caching, add wallmounted for meshes. · minetest/minetest@dd4c21c · GitHub 16:53 rubenwardy looks like it was enabled by default to start with 16:53 rubenwardy and then disabled here 16:53 rubenwardy !title https://github.com/minetest/minetest/commit/b2160bcecdecb00bb59e6ad8ece6518255dab4ad 16:53 ShadowBot rubenwardy: Disable mesh cache by default · minetest/minetest@b2160bc · GitHub 16:54 schwarzwald[m] I'd like to remove the reference counter in Irrlicht and replace it with RAII resource management, but it looks like it's not an easy thing to do. 16:54 rubenwardy #2597 16:54 ShadowBot https://github.com/minetest/minetest/issues/2597 -- Disable mesh cache by default by est31 16:55 MTDiscord There are few things in compsci more universal and consistent than the answer to whether caching improves performance: "oh geez, maybe? Like, it probably depends on a lot of factors..." 16:55 rubenwardy > this doesn't affect FPS at all, only mesh generation, which happens in another thread. 16:56 rubenwardy ok so I guess you need to check the meshgen profiler, rather than FPS differences 16:56 MTDiscord Yeah, though if you've got tight enough CPU constraints, then the meshgen thread could start colliding with the drawing one 16:56 MTDiscord In my case, though, I doubt this is happening. 16:56 MTDiscord I guess I'll revert my settings changes and conclude "it didn't help, at least for me." 16:56 rubenwardy given that it's obscure and off by default, I doubt anyone is using it 16:57 MTDiscord sounds like an excellent candidate for the axe 16:57 MTDiscord It was implemented with mesh nodes https://github.com/minetest/minetest/commit/0066bd77d25793b76fdaa9a62755cca934f0121d 16:58 MTDiscord The last time I saw meshgen as a significant performance bottleneck, I had an even bigger bottleneck caused by a PCI video card. 17:05 rubenwardy Minetest has way too many settings 17:05 rubenwardy settings increase complexity and make it harder to find bugs 17:05 rubenwardy would be good to question a bit more before adding a setting 17:06 schwarzwald[m] Are there any thoughts on the Irrlicht reference counter? 17:06 rubenwardy RAII would be a good idea. Question is whether it's worth the effort over other things, such as getting SDL in or moving from Irrlicht in general 17:07 rubenwardy using shared_ptr in IGUIElement would be nice 17:07 rubenwardy I never know whether I should ->drop() 17:19 MTDiscord More settings are fine as long as people don't actually USE them. Settings to disable features that are on by default are fine, as long as you investigate why people are turning things off if they should normally be on. Settings to enable some obscure feature are probably a suggestion that either the feature should be removed, or the default setting changed. 17:21 hmmmm [01:05 PM] Minetest has way too many settings aahhhhhhhhhhhhhhhh going the way of firefox and dumbing down the interface 17:21 hmmmm remove options at your own peril 17:22 MTDiscord hmmmm: maybe you know smth about mesh cache? 17:22 hmmmm what about it? 17:22 MTDiscord Why does it exist and can we remove it? 17:23 hmmmm because uploading meshes to the gpu is a performance bottleneck, i'm sure somebody mentioned this above 17:23 hmmmm (i haven't read the convo fully) 17:24 MTDiscord Then why is it disabled by default? 17:24 hmmmm it is??? 17:24 MTDiscord Yup https://github.com/minetest/minetest/issues/2597 17:24 hmmmm that's closed not merged... 17:25 hmmmm and it's from 2015 17:25 MTDiscord That's a trick, it was merged https://github.com/minetest/minetest/commit/b2160bcecdecb00bb59e6ad8ece6518255dab4ad 17:25 MTDiscord Yeah, it was disabled in 2015 17:25 hmmmm oh was this before github allowed for fast forward merges? 17:26 MTDiscord No clue about that 17:26 hmmmm i don't know if i was around MT during this time so i can't recall this debate very well 17:26 hmmmm the bottleneck was worst on intel integrated gpus 17:27 hmmmm see if you remove it right now you're going to harm performanse the worst on the platform it was intended for 17:27 Krock hello hmmmm. it's been a while :) 17:27 hmmmm celeron made this specifically to run on intel GMA 17:27 hmmmm hi krock 17:28 hmmmm like do you know how linux and such used to run great on older computers 17:28 hmmmm but then they started REMOVING shit 17:28 Krock with the rather recent shader additions, quite a few Intel driver bugs unrevealed 17:28 hmmmm maybe separate it out of core somehow 17:28 hmmmm but remove italtogether? god no 17:28 hmmmm ask celeron55 17:47 Desour schwarzwald[m]: irrlicht's IReferenceCounted doesn't hinder raii, you just have to use irr_ptr. (it might even be a bit faster than shared_ptr, i.e. because it's not thread-safe) 18:23 rubenwardy The mesh cache is disabled by default and there's been a few tests that show it doesn't make a difference to performance - but does impact memory usage. There's no point keeping around features with no benefit and no users 18:26 rubenwardy If tests do show it makes a difference, then it should be disabled 18:26 Desour enabled* 18:26 Desour ? 18:30 rubenwardy Yes 18:53 rubenwardy [18:27] <+hmmmm> celeron made this specifically to run on intel GMA 18:53 rubenwardy celeron55 made this over 10 years ago, it's ok to update min requirements (and he has suggested this) 19:15 hmmmm isn't there the 0.3.x MT fork still? how well maintained is it? 19:16 MTDiscord yes, its dead, if we are thinking of the same one 19:17 rubenwardy Voxeland died about 4 years ago iirc 19:20 hmmmm so what does somebody do when they have a low end vintage computer they want to play a low-resource voxel game on? 19:20 hmmmm you can't even install linux on vintage computers now because the drivers had been yanked out, so you're stuck with unmaintained branches with unpatched vulns 19:21 hmmmm same story with firefox 19:21 hmmmm and somebody can always checkout an older version and compile that..... right??? 19:21 hmmmm right?? 19:25 MisterE[m] you can always checkout stable-4 or something 19:28 rubenwardy The thing is that continuing to support ancient devices is limiting our ability to make the experience good for those with a recent device 19:30 MTDiscord I think we all agree that supporting older devices is something we all want to do, but people have very widely varying ideas of how much older. 19:31 MTDiscord Like, we've got some people who are on 10 to 15 year old computers, but then some others who are like "your machine is from 2020? Dude, why haven't you upgraded yet?!" 19:32 MTDiscord I really appreciate that MT continues to work smoothly on machines from 2010 ... but once we're talking like machines from 2005 or something, then I start to get impressed the thing can even POST, and stop expecting to find software that actually runs on it... 19:32 rubenwardy I believe we decided on a cutoff of high end 12 years ago 19:32 rubenwardy Which was 2010 19:33 MTDiscord Hmm, that seems fine, but I guess that means I've only got about 2 more years worth of Minetest left 😅 19:42 MisterE[m] " Hmm, that seems fine,..." <- well, its not guranteed to stop working then. 20:00 schwarzwald[m] "schwarzwald: irrlicht's IReferen..." <- I need to invoke `drop` at some point, and I'd rather use STL types to keep Irrlicht more focused on graphics and not on maintaining its own shared pointer implementation. 20:01 schwarzwald[m] My current thought is to introduce a wrapper object that calls `drop` on the resource after it goes out of scope, so that I can introduce nicer lifetime management gradually instead of having to make huge changes all at once. 20:01 Desour as I said, use irr_ptr 20:02 Desour it calls drop 20:02 schwarzwald[m] > rather use STL types 20:02 Desour unique_ptr with a deleter that calls drop would probably also work, but it wouldn't be copyable 20:03 schwarzwald[m] My current interface uses a `unique_ptr` and just doesn't call `drop`. As far as I can tell there are no ill effects of not calling `drop` except that Irrlicht will think it wasn't freed. 20:04 schwarzwald[m] Optimally I'd rather not have multiple owners of a resource if I can avoid it, hence `unique_ptr`. 20:05 schwarzwald[m] If I need to pass it somewhere non-owning I'll just pass the raw pointer. 20:07 Desour one problem is that currently raw ptrs are passed and you don't know if the callee takes ownership 20:08 schwarzwald[m] Yes, that is indeed a problem. 20:08 Desour using unique_ptrs where it makes sense is probably a good thing in the long run, but if you want to do gradual changes, irr_ptr is easier. (and performance-wise the refcounter is probably not that big of a problem) 20:09 schwarzwald[m] I added a new method instead of changing the interface of the old one, so I can introduce the unique_ptr strat in one place at a time. 20:09 schwarzwald[m] Maybe I should focus on that rn instead of trying to refactor the OBJ mesh loader, which was my original purpose. I get distracted easily, haha. 20:09 Desour we should move irr_ptr to the irrlicht repo btw. 20:09 schwarzwald[m] irr_ptr isn't in the irrlicht repo? 20:10 Desour no, it's from minetest 20:11 Desour > I get distracted I know that feeling when you see raw ptrs everywhere near the code you touch, then you replace them with smart ptrs, and never come to an end 20:11 schwarzwald[m] What actually happens if I just call the destructor and don't drop it from the reference counter? As I mentioned, I do that in my test suite, and it seems to just work. 20:12 rubenwardy potentially a double free 20:12 Desour just deleting is fine, iff the thing is referenced nowhere else. 20:13 rubenwardy unique_ptr and shared_ptr have different semantics, don't abuse a unique_ptr where a shared_ptr would make more sense 20:13 schwarzwald[m] Desour: Somewhere way back where this all started, I'm still trying to fix the memory leak Coverity detected lol. 20:15 Desour oh, can I get a link to more info about the leak? 20:15 schwarzwald[m] It claims the mesh buffer allocated in SObjMtl (temporary struct object in the OBJ loader) is getting leaked. 20:16 schwarzwald[m] When I looked at it, it looked like the meshbuffer ownership gets transferred to the Mesh object, and way later the Mesh object correctly frees the meshbuffer, but of course who knows where all that pointer ends up and what happens to it inbetween. 20:18 Desour i see 20:19 schwarzwald[m] Also unclear from that how serious a leak that would be. I assume not very serious, or we would've already tried to fix it. 20:19 schwarzwald[m] But I like trying to fix stuff. 20:21 schwarzwald[m] I'd like to push something small, but I have to figure out what exactly is small enough to be harmless and easy to review, but big enough to actually be something worthwhile. 20:23 schwarzwald[m] I guess I need to limit the scope of the change to just introducing the `unique_ptr` or something. 20:25 Desour I'd just use smart ptrs for everything in the whole file (+header), then adjust other code to use the new interface by calling .release() (for createMesh()) 20:26 Desour => limited in scope to file, enough of a change, because it's a whole file 20:27 schwarzwald[m] The reason I added a new method instead of changing the old one is that I didn't want to change all the other mesh loaders at once. xD 20:27 schwarzwald[m] But maybe I should. 20:27 schwarzwald[m] That would require changing the method signature in the base class as well as all mesh loader implementations at once. 20:27 schwarzwald[m] = more work, and I already am spending too much time on this lol 20:28 Desour createMesh() isn't called in minetest. and the compiler will show you all code instances that you need to change 20:31 schwarzwald[m] True, maybe there won't be very many. 20:32 schwarzwald[m] The tricky part might be figuring out where ownership of the Mesh gets transferred in the rest of the code. xD 20:32 schwarzwald[m] But that should be possible with a little investigating. 20:33 schwarzwald[m] I guess I should remove my test suite for now? I was just starting it and it hardly tests anything at this point, + adding a dependency. 20:34 schwarzwald[m] I went and made an in-memory implementation of IReadFile so that I could write OBJ data in source code and pass it as a "file" to the loader. :( 20:40 Desour btw. FYI, you can't just use a unique_ptr for SObjMtl::Meshbuffer, as it's grabbed by the mesh and also owned in the Materials irr::array 20:41 Desour if you want to add irr_ptr to irrlicht without causing API or ABI compatibility, you can put it inside the irr namespace, and later do using irr_ptr = irr::irr_ptr; in minetest 20:43 schwarzwald[m] Ah, no, I'm using the unique_ptr for the Mesh object currently. 20:44 schwarzwald[m] I was thinking I might change the design of SObjMtl entirely because I didn't understand why it was done that way. That's why I started writing tests so I could start from scratch. 20:45 schwarzwald[m] i.e. maybe do away with SObjMtl completely 20:45 schwarzwald[m] The createMesh function is over 200 lines or something, which seems a little overkill.