| Time |
Nick |
Message |
| 00:03 |
|
diceLibrarian joined #minetest-dev |
| 00:15 |
|
proller joined #minetest-dev |
| 00:25 |
Zughy[m] |
I don't understand why it only affects Android but thank you savilli, it worked |
| 00:36 |
MTDiscord |
<savilli> Because android is the most different setup |
| 00:37 |
MTDiscord |
<savilli> It could affect everything except Android, but you would notice it and do smth sooner ofc |
| 02:07 |
|
diceLibrarian joined #minetest-dev |
| 04:00 |
|
MTDiscord joined #minetest-dev |
| 04:25 |
|
calcul0n_ joined #minetest-dev |
| 05:53 |
|
YuGiOhJCJ joined #minetest-dev |
| 07:02 |
|
Fixer joined #minetest-dev |
| 08:24 |
sfan5 |
bad param2 means what? |
| 08:32 |
|
pgimeno joined #minetest-dev |
| 08:43 |
|
appguru joined #minetest-dev |
| 09:44 |
|
olliy joined #minetest-dev |
| 11:38 |
|
appguru joined #minetest-dev |
| 12:37 |
|
appguru joined #minetest-dev |
| 13:32 |
|
Taoki joined #minetest-dev |
| 13:39 |
|
proller joined #minetest-dev |
| 15:57 |
|
appguru joined #minetest-dev |
| 16:12 |
|
beanzilla joined #minetest-dev |
| 16:17 |
|
beanzilla joined #minetest-dev |
| 16:23 |
MTDiscord |
<savilli> It means > 5 for wallmounted nodes |
| 16:26 |
|
appguru joined #minetest-dev |
| 16:26 |
MTDiscord |
<Warr1024> 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 |
<Warr1024> 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 |
<savilli> 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 |
<savilli> 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 |
<Warr1024> 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 |
<savilli> 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 |
<savilli> The feature looks very old and not popular. I wonder if I should fix it or just remove it. |
| 16:41 |
MTDiscord |
<Warr1024> 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 |
<Warr1024> 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 |
<savilli> It's only for mesh nodes |
| 16:42 |
MTDiscord |
<Warr1024> Odd that it would be tied to smooth lighting |
| 16:42 |
MTDiscord |
<savilli> Because you can't cache a mesh with smooth lighting |
| 16:42 |
MTDiscord |
<savilli> You need to change it every time |
| 16:42 |
MTDiscord |
<Warr1024> I thought you'd need to change it every time with unsmooth lighting too. |
| 16:42 |
MTDiscord |
<Warr1024> I can try turning off smooth lighting and see if it makes any difference, at least. |
| 16:43 |
MTDiscord |
<Warr1024> 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 |
<savilli> 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 |
<savilli> Yes |
| 16:44 |
rubenwardy |
sounds like a removal candidate :) |
| 16:44 |
MTDiscord |
<Warr1024> 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 |
<savilli> I feel I have too high-end hardware to test that. Maybe it does for someone. |
| 16:45 |
MTDiscord |
<Warr1024> 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 |
<Warr1024> 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 |
<Warr1024> 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 |
<savilli> Interesting that smooth lighting is disabled for android by default |
| 16:47 |
MTDiscord |
<Warr1024> I found smooth_lighting = false and enable_mesh_cache = true ... is that enough to enable it? |
| 16:47 |
MTDiscord |
<savilli> Yup |
| 16:48 |
rubenwardy |
is enable_mesh_cache still disabled on Android though? |
| 16:48 |
MTDiscord |
<savilli> 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 |
<savilli> 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 |
<Warr1024> 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/minetestb2160bc · 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 |
<Warr1024> 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 |
<Warr1024> Yeah, though if you've got tight enough CPU constraints, then the meshgen thread could start colliding with the drawing one |
| 16:56 |
MTDiscord |
<Warr1024> In my case, though, I doubt this is happening. |
| 16:56 |
MTDiscord |
<Warr1024> 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 |
<Warr1024> sounds like an excellent candidate for the axe |
| 16:57 |
MTDiscord |
<savilli> It was implemented with mesh nodes https://github.com/minetest/minetest/commit/0066bd77d25793b76fdaa9a62755cca934f0121d |
| 16:58 |
MTDiscord |
<Warr1024> 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 |
<Warr1024> 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] <rubenwardy> 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 |
<savilli> hmmmm: maybe you know smth about mesh cache? |
| 17:22 |
hmmmm |
what about it? |
| 17:22 |
MTDiscord |
<savilli> 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 |
<savilli> Then why is it disabled by default? |
| 17:24 |
hmmmm |
it is??? |
| 17:24 |
MTDiscord |
<savilli> 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 |
<savilli> That's a trick, it was merged https://github.com/minetest/minetest/commit/b2160bcecdecb00bb59e6ad8ece6518255dab4ad |
| 17:25 |
MTDiscord |
<savilli> Yeah, it was disabled in 2015 |
| 17:25 |
hmmmm |
oh was this before github allowed for fast forward merges? |
| 17:26 |
MTDiscord |
<savilli> 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:30 |
|
Desour joined #minetest-dev |
| 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 |
<Jonathon> 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 |
<Warr1024> 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 |
<Warr1024> 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 |
<Warr1024> 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 |
<Warr1024> 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] |
<MTDiscord> "<Warr1024> Hmm, that seems fine,..." <- well, its not guranteed to stop working then. |
| 20:00 |
schwarzwald[m] |
<Desour> "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:40 |
|
Noisytoot joined #minetest-dev |
| 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. |
| 21:35 |
|
Desour joined #minetest-dev |
| 22:35 |
|
panwolfram joined #minetest-dev |
| 22:49 |
|
diceLibrarian joined #minetest-dev |
| 22:52 |
|
proller joined #minetest-dev |
| 23:16 |
|
proller joined #minetest-dev |
