Time |
Nick |
Message |
00:31 |
|
kahrl joined #minetest-dev |
00:41 |
|
shadowzone joined #minetest-dev |
00:50 |
|
Chicken_shadow joined #minetest-dev |
00:56 |
|
Zeno` joined #minetest-dev |
01:02 |
|
paramat joined #minetest-dev |
01:13 |
|
chchjesus joined #minetest-dev |
01:20 |
paramat |
hmmmm, mgv5 makechunk time is 110ms with 1 eased noise (terrain only), 150ms with all 5 noises eased, 130ms with 3 eased noises (terrain, cave1, cave2) (10ms per easing). any preferences? i think 3 eased is a good balance, being faithfull to the original's caves but not easing dirt/lava blob shape and wetness |
01:22 |
paramat |
https://cdn.mediacru.sh/eVCExlDvoe29.png |
01:25 |
kahrl |
hmmmm: did c55 agree with that license change? |
01:27 |
kahrl |
and I guess PilzAdam would have to agree too as he contributed the faster version of easeCurve |
01:29 |
kahrl |
proller also changed a tiny bit |
01:40 |
|
mos_basik joined #minetest-dev |
01:41 |
Zeno` |
Hi kahrl. I had to manually apply that commit in my refactor_the_game branch (like you warned me lol) |
01:42 |
Zeno` |
But I didn't know how to make it look like you wrote it :( |
01:44 |
kahrl |
no worries |
01:45 |
Zeno` |
I referenced your commit though |
01:45 |
kahrl |
(it'll remain in the minetest/master commit history regardless, since it's already been pushed) |
01:45 |
Zeno` |
yep, was just letting you know. I wanted to "credit" you with it, I just couldn't work out how |
01:47 |
kahrl |
the way to do that would probably be via the --author option of git commit |
01:47 |
kahrl |
but don't worry about it |
01:47 |
Zeno` |
ok |
02:07 |
paramat |
i think the license of c55's mgv5 branch might just have been the result of copy-paste, it seems to need editing with credit also to c55 etc. |
02:11 |
hmmmm |
kahrl, of course. |
02:12 |
hmmmm |
with regard easeCurve(), that was my very first contribution to minetest |
02:12 |
hmmmm |
pilzadam did commit it for me and all |
02:12 |
paramat |
ah okay |
02:12 |
hmmmm |
proller removed all of his noise-related modificiations when he rage quit |
02:12 |
hmmmm |
so it's me and celeron basically. |
02:15 |
OldCoder |
I had a memory corruption crash just now. Obviously no way to trace it. But are occasional such corruptions known to occur? |
02:15 |
OldCoder |
I silvercrab.log (Modified) Row 10783 Col 24 7:08 Ctrl-K H for help |
02:15 |
OldCoder |
*** glibc detected *** minetestserver: malloc(): memory corruption (fast): |
02:16 |
OldCoder |
This sort of error; has it been seen sometimes? |
02:54 |
Zeno` |
ShadowNinja, I have amended the brace issue, answered your question, and changed all of the sizeof operators to not use () unless necessary |
02:54 |
Zeno` |
I find sizeof(object) as annoying as return(object) |
02:55 |
Zeno` |
As for the function like macro expansion... see line note ;) |
03:11 |
|
zat joined #minetest-dev |
03:37 |
Zeno` |
Someone should join linuxgaming and watch their RAM usage go up without ever stopping |
03:38 |
Zeno` |
just walk around doing nothing and watch it rise and rise... I've just hit 5GB usage in.. hmmm about 8 minutes |
03:39 |
Zeno` |
make that 6GB now |
03:42 |
Zeno` |
and 7 |
03:48 |
|
hmmmm joined #minetest-dev |
04:12 |
|
paramat left #minetest-dev |
05:03 |
|
Miner_48er joined #minetest-dev |
05:28 |
Zeno` |
I have bad news :( |
05:28 |
Zeno` |
The mem leak... can't find it |
05:29 |
Zeno` |
It happens on all servers not just linuxgaming as I thought before; it's just that it seems to happen faster on linuxgaming |
05:30 |
Zeno` |
bisect time I guess |
05:34 |
hmmmm |
mem leak? |
05:35 |
hmmmm |
valgrind no workie? |
05:35 |
|
sol_invictus joined #minetest-dev |
05:35 |
Zeno` |
yes, client mem usage just keeps going up and up and up. valgrind shows nothing interesting |
05:35 |
hmmmm |
i'm willing to bet it's my own shitty code to blame |
05:35 |
Zeno` |
it probably wouldn't if the mem was finally released anyway |
05:35 |
Zeno` |
I'll do the bisect soon |
05:36 |
hmmmm |
did you check to see if it's caused by VBOs |
05:37 |
Zeno` |
not yet |
05:37 |
Zeno` |
I wanted to confirm if it was actually happening and not my imagination first |
05:38 |
Zeno` |
Would VBOs make normal RAM increase? |
05:39 |
Zeno` |
I suppose they could depending on what opengl does with them |
05:39 |
|
chchjesus joined #minetest-dev |
05:39 |
hmmmm |
yes |
05:39 |
hmmmm |
VBOs cause a dramatic increase in memory usage, the price to pay for an incredible FPS boost |
05:39 |
hmmmm |
not by their very nature, but because of irrlicht sucking ass |
05:40 |
Zeno` |
ok, will keep that in mind while searching |
05:40 |
Zeno` |
it's just funny that I've never noticed this before... i.e. I would have been checking RAM way back... |
05:40 |
Zeno` |
umm... can't find the commit |
05:40 |
hmmmm |
having VBOs enabled will make the client look like it's leaking memory, but it isn't, i promise. |
05:43 |
Zeno` |
I can disable them? |
05:48 |
hmmmm |
yeah you should be able to in the config, i forget the name of the setting |
06:06 |
Zeno` |
stupid irrlicht |
06:07 |
Zeno` |
since the client is using 15GB of RAM I'd really want fps to be 1000 hehehe |
06:08 |
hmmmm |
crazy! |
06:08 |
hmmmm |
again, we're not sure if this is the problem or not |
06:12 |
Zeno` |
hmmmm, you've made a terrible mistake! https://github.com/minetest/minetest/commit/bc28ca0636f5bce85683a7114ad99df4e7791847#diff-5c9fad38a1e2b7a0227fd3f5282dcc09R534 You're missing a tab |
06:13 |
hmmmm |
what do you mean |
06:13 |
Zeno` |
needs two tabs! Da style! |
06:13 |
hmmmm |
for line continuations? |
06:13 |
Zeno` |
apparently. I think |
06:14 |
hmmmm |
oh, I never knew that |
06:14 |
hmmmm |
i always did one tab |
06:15 |
Zeno` |
Maybe it doesn't count for line continuation but just when breaking conditionals |
06:16 |
hmmmm |
i don't know man. |
06:16 |
Zeno` |
neither do I |
06:16 |
Zeno` |
it's... a conundrum |
06:16 |
hmmmm |
I just added a lot of flack to a major .h file |
06:17 |
Zeno` |
Despite this VBO complication I might keep hunting for a bit, just in case |
06:19 |
Zeno` |
Btw, I was joking (mostly) about the missing tab |
06:20 |
Zeno` |
My sense of humour if definitely warped |
06:20 |
hmmmm |
oh |
06:21 |
Zeno` |
I think many people do not "get" (some) Australian's sense of humour. Gets me in trouble. I will try and refrain |
06:21 |
hmmmm |
you're australian? |
06:23 |
Zeno` |
I am |
06:36 |
|
nore joined #minetest-dev |
06:50 |
|
darkrose joined #minetest-dev |
06:54 |
|
hintss joined #minetest-dev |
06:59 |
Zeno` |
a fellow Australian |
07:03 |
RealBadAngel |
https://github.com/minetest/minetest/pull/1778 |
07:03 |
RealBadAngel |
hi |
07:05 |
RealBadAngel |
hmmmm, can you take a look? |
07:05 |
RealBadAngel |
ShadowNinja, above adds wallmounted for meshnodes as you wanted |
07:06 |
RealBadAngel |
theres also config option to enable/disable mesh caching at startup |
07:16 |
Zeno` |
RealBadAngel, https://github.com/minetest/minetest/pull/1777 |
07:17 |
Zeno` |
Also: https://github.com/minetest/minetest/pull/1776 |
07:17 |
|
kilbith joined #minetest-dev |
07:19 |
Zeno` |
RealBadAngel, also (for testing): https://github.com/minetest/minetest/pull/1756 |
07:20 |
RealBadAngel |
im ok with 1776 and 1777, but why removing brackets in sizeof ? |
07:20 |
Zeno` |
same reason as you don't do return(0) (sizeof is an operator not a function) |
07:21 |
Zeno` |
but I can revert those sizeof changes if it's a big deal |
07:21 |
RealBadAngel |
i was never thinkin of it this way, brackets seems here quite natural, thats all |
07:21 |
Zeno` |
I guess |
07:21 |
Zeno` |
I dunno... |
07:22 |
Zeno` |
it's probably a pet peeve of mine and if it stops something being merged I should get over it |
07:23 |
RealBadAngel |
whatever SN decides |
07:24 |
RealBadAngel |
hes more into code style than me |
07:25 |
RealBadAngel |
btw, about memory usage, caching meshes can take up to 0.5gb in case of dreambuilder |
07:25 |
RealBadAngel |
so i decided to add option to disable caching for systems with less ram |
07:26 |
Zeno` |
I think I'll apply your patch and test |
07:27 |
RealBadAngel |
caching is disabled by default |
07:27 |
RealBadAngel |
enable_mesh_cache = true to enable it |
07:28 |
Zeno` |
hmm, how do I get the raw git diff again? |
07:31 |
|
Hunterz joined #minetest-dev |
07:32 |
|
ImQ009 joined #minetest-dev |
07:48 |
|
Amaz joined #minetest-dev |
08:00 |
|
chchjesus joined #minetest-dev |
08:01 |
Zeno` |
RealBadAngel, tested #1778 playing game (minimal RAM usage increase as you mentioned) and I noticed it seemed smoother. Also checked with valgrind with no issues. So without a formal code review it looks fine |
08:01 |
ShadowBot |
https://github.com/minetest/minetest/issues/1778 -- Add option to enable mesh caching, add wallmounted for meshes. by RealBadAngel |
08:20 |
|
chchjesus__ joined #minetest-dev |
08:23 |
|
chchjesus__ joined #minetest-dev |
08:27 |
|
chchjesus joined #minetest-dev |
08:33 |
|
Amaz joined #minetest-dev |
09:17 |
|
FR^2 joined #minetest-dev |
09:22 |
Zeno` |
Why are there commits happening without pull requests when there are 107 pull requests? |
09:27 |
sol_invictus |
I wonder too, Zeno |
09:38 |
kilbith |
because the commits without PR comes from the official devs |
09:38 |
kilbith |
which are trustable |
09:38 |
|
GrimKriegor joined #minetest-dev |
09:40 |
Zeno` |
well you'd hope so |
09:41 |
Zeno` |
but... don't the core devs need approval from another core dev as well? |
09:41 |
Zeno` |
That is what I was led to believe |
09:41 |
|
VargaD_ joined #minetest-dev |
09:41 |
Zeno` |
And doesn't answer the question why there are 107 pull requests just left to rot |
10:02 |
|
jin_xi joined #minetest-dev |
10:12 |
|
ImQ009 joined #minetest-dev |
10:13 |
kilbith |
Zeno` : ask to proller |
10:40 |
|
CraigyDavi joined #minetest-dev |
10:47 |
|
rubenwardy joined #minetest-dev |
10:51 |
|
CraigyDavi` joined #minetest-dev |
10:54 |
|
CraigyDavi`` joined #minetest-dev |
10:59 |
jin_xi |
ShadowNinja: i do not agree with your point regarding lag for https://github.com/minetest/minetest/pull/1737 |
10:59 |
jin_xi |
its not a problem and users do deal with the very same lag for many gui interactions |
11:01 |
jin_xi |
also it adds a very useful feature with a lot of potential for many mods |
11:05 |
Zeno` |
yeah but the spacing/formatting is all wrong :( |
11:06 |
jin_xi |
that is fixable with some helpful comments on the pull ;) |
11:07 |
Zeno` |
well, it looks good to me *shrug* |
11:08 |
Zeno` |
I don't think lag is an issue either... it's already an issue in lots of other places as you pointed out |
11:09 |
jin_xi |
also its not to bad, i made a tetris in a formspec mod and its absolutely playable in single player |
11:09 |
jin_xi |
some lag, some lag spikes, but no reason not to do such things |
12:00 |
OldCoder |
*** glibc detected *** minetestserver: malloc(): memory corruption (fast): This sort of error; has it been seen sometimes? |
12:26 |
|
proller joined #minetest-dev |
12:35 |
|
shadowzone joined #minetest-dev |
12:45 |
proller |
still no answer why https://github.com/minetest/minetest/commit/d274cbfce6ed39f5b7ad41261ede8c0fad7e980a was commited and who was approsers |
12:45 |
sfan5 |
proller: what is the problem with that commit? |
12:45 |
sfan5 |
if you don't like it just revert it in freeminer |
12:46 |
sfan5 |
it's not like you are doing anything for minetest |
12:46 |
proller |
but minetest have rules, but not for all |
12:54 |
Zeno` |
nobody approbed it |
12:54 |
Zeno` |
just like the last 3 commits were not approved |
12:55 |
Zeno` |
sfan5, the problem is what I stated earlier |
12:55 |
Zeno` |
Zeno`> Why are there commits happening without pull requests when there are 107 pull requests? |
12:55 |
Zeno` |
<sol_invictus> I wonder too, Zeno |
12:57 |
Zeno` |
there is no problem (that I can find) with those commits, but it's... it's the "method" (or lack of) that annoys me |
12:59 |
Zeno` |
you can't say "this is the rule" but I am exempt |
13:09 |
|
rubenwardy joined #minetest-dev |
13:22 |
|
ImQ009 joined #minetest-dev |
13:27 |
|
kahrl joined #minetest-dev |
13:28 |
|
CraigyDavi joined #minetest-dev |
13:28 |
kahrl |
Zeno`: subsystem maintainers can push commits to their subsystem without approval |
13:28 |
kahrl |
hmmmm is the mapgen subsystem maintainer |
13:29 |
Zeno` |
ahh ok, that makes sense |
13:30 |
kahrl |
http://dev.minetest.net/Organisation <-- list of subsystems |
13:36 |
|
PenguinDad joined #minetest-dev |
13:44 |
|
Amaz left #minetest-dev |
14:09 |
kahrl |
Zeno`: #1644 looks quite good |
14:09 |
ShadowBot |
https://github.com/minetest/minetest/issues/1644 -- Refactor main by Zeno- |
14:09 |
kahrl |
I have some time today to discuss the remaining TODOs/FIXMEs, if you want |
14:12 |
Zeno` |
thanks. Can we catch up tomorrow? It's 12:12AM here and I was just about to sleep |
14:12 |
Zeno` |
I can be around whenever you might be on next |
14:13 |
kahrl |
sure, although I'm not sure yet when I'll be online |
14:13 |
Zeno` |
Discussion is the best option |
14:13 |
Zeno` |
ok, but I will hang around all day tomorrow (I don't mind doing that)? |
14:13 |
Zeno` |
or email me! :) |
14:13 |
Zeno` |
I will PM you my email address |
14:14 |
Zeno` |
sorry for not wanting to do it now, I'm just very tired :( |
14:14 |
kahrl |
I understand |
14:15 |
kahrl |
have a good sleep :) |
14:17 |
Zeno` |
thanks |
14:17 |
Zeno` |
I really wish I could stay awake, but I make bad decisions at this time of night anyway heh |
14:18 |
kahrl |
hehe |
14:29 |
|
jin_xi joined #minetest-dev |
14:58 |
|
hmmmm joined #minetest-dev |
15:09 |
|
zat joined #minetest-dev |
15:20 |
|
hintss joined #minetest-dev |
15:33 |
|
Amaz joined #minetest-dev |
15:45 |
|
shadowzone joined #minetest-dev |
15:56 |
hmmmm |
for my upcoming unified map generation component interface, one of the goals is to have a single CRUD interface |
15:56 |
hmmmm |
the update and delete parts are a bit more difficult |
15:58 |
hmmmm |
not a problem though, thanks to the wonder atomic swaps... which got me thinking, since it's not really that much of an issue to modify registered entities after the world has started, would anybody have a use for changing nodes and item definitions? |
16:15 |
|
rubenwardy joined #minetest-dev |
16:16 |
|
CraigyDavi` joined #minetest-dev |
16:24 |
sol_invictus |
yeah, changing nodes can be useful |
16:24 |
sol_invictus |
hmmmm: something like more advanced circular saw can get use of that |
16:30 |
|
CraigyDavi`` joined #minetest-dev |
16:40 |
rubenwardy |
hmmmm, Wouldn't changing a node definition mean that all nodes that use it will be changed? How would that be used? |
16:44 |
|
PilzAdam joined #minetest-dev |
16:48 |
sfan5 |
rubenwardy: changing a nodedef would affect all nodes |
16:51 |
|
Calinou joined #minetest-dev |
17:05 |
hmmmm |
yup |
17:05 |
hmmmm |
so I don't know |
17:05 |
hmmmm |
only thing is, the server would need to resend nodedefs to the client |
17:06 |
hmmmm |
this could be useful for season changing or something... imagine all default:dirt_with_grass changing to default:dirt_with_snow instantaneously |
17:11 |
rubenwardy |
Add notice when only minimal is installed - https://github.com/minetest/minetest/pull/1743 |
17:15 |
|
zat joined #minetest-dev |
17:15 |
|
nore joined #minetest-dev |
17:17 |
|
zat joined #minetest-dev |
17:23 |
|
ImQ009 joined #minetest-dev |
17:35 |
rubenwardy |
Only set the camera update keymap when using a debug build - https://github.com/minetest/minetest/pull/1744 |
17:36 |
rubenwardy |
Any comments? Would you rather I removed it entirely for non-debug builds? |
17:42 |
hmmmm |
the problem is that it's useless |
17:42 |
hmmmm |
any cheater can just build their client for debug and then they can do the same thing |
17:42 |
hmmmm |
there's no way we're going to fix visibility cheats so don't even bother IMO.. just accept it as it is |
17:42 |
rubenwardy |
Not useless |
17:43 |
rubenwardy |
Stops noob confusion if they accidentally hit the key |
17:43 |
rubenwardy |
that's the #1 reason |
17:43 |
hmmmm |
well |
17:43 |
hmmmm |
a lot of superfluous debug information is available to the end user on the client |
17:44 |
hmmmm |
why not add a config setting for debug mode that allows access to camera update and the more "hardcore" debugging menus |
17:45 |
hmmmm |
i don't like tieing certain features from working to the build type, that is a source for problems |
17:45 |
rubenwardy |
I made some code that only allowed camera update to be disabled in debug (F5) mode |
17:46 |
ShadowNinja |
Um, why is it that every object that get_objects_in_radius returns is a LuaEntity with the name "singleplayer", even though I know they're all supposed to be "worldedit:..."? Can someone else confirm? |
17:48 |
ShadowNinja |
This is with a fixed /we-clearobjects that calculates the size correctly (Just sqrt(x^2+y^2+z^2) instead of sqrt(x*2+y*2+z*2) or whatever it was). |
17:50 |
ShadowNinja |
http://pastebin.ubuntu.com/8722075/ |
17:52 |
PenguinDad |
That would explain why my attempt to fix the player deletion with /we-clearobjects didn't work… |
17:52 |
|
VanessaE joined #minetest-dev |
17:54 |
ShadowNinja |
PenguinDad: I fixed the player deletion bug (obj:is_player() still works) but now that the deletion box is calculated properly it includes the selection boxes and I con't exclude those from deletion. |
17:57 |
PenguinDad |
ShadowNinja: I tried to fix it a few months ago |
17:58 |
ShadowNinja |
obj:get_entity_name() works though... |
17:58 |
ShadowNinja |
I might have broken this with my coroutine fixes... |
18:05 |
ShadowNinja |
minetest.luaentities is broken too. |
18:11 |
|
shadowzone joined #minetest-dev |
18:12 |
ShadowNinja |
The metatable in minetest.registered_entities is fine, it's being explicitly overridded in the entity. |
18:12 |
RealBadAngel |
ShadowNinja, updated https://github.com/minetest/minetest/pull/1778 |
18:15 |
ShadowNinja |
RealBadAngel: Looks good. |
18:16 |
hmmmm |
HRRMMM. |
18:17 |
hmmmm |
looking for a C++03 way of statically populating a std::map |
18:18 |
ShadowNinja |
hmmmm: I don't think there is one. |
18:19 |
ShadowNinja |
If it'll stay static a const array might work, but if it's just initial values you'll have to add them one by one with operator[] (or equivalent).. |
18:20 |
hmmmm |
i thought so! :( |
18:21 |
|
shadowzone joined #minetest-dev |
18:22 |
Calinou |
don't all current LTS operating systems support C++11 now? |
18:23 |
hmmmm |
maybe it does |
18:23 |
hmmmm |
i'm mostly looking at freebsd 9.x and earlier |
18:23 |
hmmmm |
there are people who still use that shit, like myself |
18:25 |
PilzAdam |
Calinou, ubuntu 12.04 doesn't, AFAIK |
18:26 |
Calinou |
that is not the current LTS |
18:26 |
Calinou |
14.04 is |
18:26 |
Calinou |
if you host game servers I hope you can upgrade every 2 years… |
18:26 |
Calinou |
:\ |
18:26 |
RealBadAngel |
hmmmm, are you ok with my pull? |
18:26 |
PilzAdam |
I though by "current" you meant "supported" |
18:26 |
PilzAdam |
+t |
18:30 |
hmmmm |
what pull |
18:31 |
RealBadAngel |
#1778 |
18:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/1778 -- Add option to enable mesh caching, add wallmounted for meshes. by RealBadAngel |
18:31 |
PenguinDad |
What about #1779 ? |
18:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/1779 -- Fix indentation issues of fd5eaae by PenguinDad |
18:31 |
|
Miner_48er joined #minetest-dev |
18:33 |
ShadowNinja |
PenguinDad: <3 +1 |
18:33 |
PenguinDad |
I just couldn't leave these unfixed :P |
18:34 |
ShadowNinja |
PenguinDad: I'm working on a bug at the moment, but that's simple enough that anyone can merge it. |
18:38 |
|
Krock joined #minetest-dev |
18:43 |
|
rubenwardy joined #minetest-dev |
18:55 |
|
kaeza joined #minetest-dev |
19:10 |
|
CraigyDavi` joined #minetest-dev |
19:26 |
hmmmm |
RealBadAngel, in that pull request you clone and then subsequently drop a mesh inside of mapblock_mesh_generate_special |
19:26 |
hmmmm |
that can't be too good for performance... |
19:28 |
Calinou |
wtf |
19:29 |
Calinou |
you're accepting a commit purely done for styling fixes? |
19:29 |
Calinou |
you said it was a no-go for comments |
19:33 |
ShadowNinja |
Calinou: Huh? |
19:33 |
Calinou |
you're accepting a style commit; which may break PRs |
19:34 |
Calinou |
why would you refuse comment PR then |
19:37 |
ShadowNinja |
Calinou: It fixes code that has no active PRs. Fixing code style over the entire project is still a no-go. |
19:37 |
Calinou |
ah |
19:38 |
|
iqualfragile joined #minetest-dev |
19:39 |
ShadowNinja |
PenguinDad: Fixed that bug, pushed. |
20:07 |
|
shadowzone joined #minetest-dev |
20:09 |
|
ImQ009 joined #minetest-dev |
20:34 |
|
shadowzone joined #minetest-dev |
21:04 |
|
iqualfragile_ joined #minetest-dev |
21:06 |
|
proller joined #minetest-dev |
21:35 |
|
Amaz joined #minetest-dev |
22:14 |
|
shadowzone joined #minetest-dev |
22:23 |
|
zat joined #minetest-dev |
22:30 |
Megaf |
I wish I could compile minetest on OS X as easy I can on GNU/Linux |
22:32 |
RealBadAngel |
hmmmm, when no longer needed (added to mesh collector) it is dropped |
22:33 |
RealBadAngel |
thats how it works. anyway this is only case of cache disabled, slower one |
22:33 |
kilbith |
RBA, is it possible to have collisionbox perfectly adjusted to the models ? |
22:33 |
RealBadAngel |
no. look at the name |
22:33 |
kilbith |
boxes yeah |
22:33 |
kilbith |
just curiosity |
22:47 |
|
zat joined #minetest-dev |
22:52 |
RealBadAngel |
kahrl, here? |
22:54 |
kahrl |
RealBadAngel: just came back here |
22:54 |
kahrl |
what's up? |
22:55 |
RealBadAngel |
need a vote, #1778 |
22:55 |
ShadowBot |
https://github.com/minetest/minetest/issues/1778 -- Add option to enable mesh caching, add wallmounted for meshes. by RealBadAngel |
22:56 |
kahrl |
https://github.com/RealBadAngel/minetest/commit/b3b69809ab80032167f456e88609b48d9a9d766b#diff-c03ca828c6b8a7695f2cd7e52c316a3cR1751 |
22:56 |
kahrl |
that looks like a leak |
22:58 |
kahrl |
also, is there a way to "not repeat yourself" in that part of the code? (it's almost the same as the branch above) |
23:00 |
RealBadAngel |
its not a leak. without cache only 0 mesh is stored. |
23:01 |
kahrl |
how is it not a leak? |
23:01 |
RealBadAngel |
when you need it rotated you have to make a clone of 0, apply transformations, add it to collector, then wipe |
23:01 |
kahrl |
in line 1751 you allocate a SMesh and in line 1752 you overwrite the only pointer to it with a different one |
23:02 |
RealBadAngel |
clone does no overwrite pointers |
23:02 |
RealBadAngel |
*not |
23:02 |
kahrl |
but = does |
23:02 |
Megaf |
Thats a nice error, isnt it? "PANIC: unprotected error in call to Lua API (not enough memory)" |
23:03 |
Megaf |
caused by a dramatic increased on teh active block range by me |
23:03 |
RealBadAngel |
clone takes a pointer to empty mesh, and adds to it source mesh (buffer by buffer) |
23:03 |
kahrl |
huh |
23:04 |
kahrl |
in line 1752 cloneMesh clearly only takes one parameter, the source mesh |
23:04 |
kahrl |
and presumably it returns a pointer to the newly allocated cloned mesh |
23:04 |
RealBadAngel |
hmmz |
23:05 |
kahrl |
Megaf: so... don't do that then? :) |
23:05 |
RealBadAngel |
lemme think a while |
23:06 |
RealBadAngel |
i think youre right, will fix it |
23:07 |
kahrl |
RealBadAngel: also if you change the type to scene::IMesh* you can remove the cast in line 1752, I think |
23:08 |
RealBadAngel |
but about "not repeating yourself", using only one append would require another check |
23:08 |
RealBadAngel |
so code will be slower |
23:09 |
RealBadAngel |
after first bunch of checks i will have to recheck for existence of pointer |
23:11 |
kahrl |
well if the cache is disabled the code will be slow anyway |
23:12 |
RealBadAngel |
no point to make it even slower |
23:13 |
RealBadAngel |
also such extra check will hit cached one too |
23:13 |
RealBadAngel |
it is already slowed down by checking for the pointer |
23:14 |
RealBadAngel |
which shouldnt take place at all |
23:14 |
RealBadAngel |
but it is here for lazy modders |
23:16 |
RealBadAngel |
some things have right to crash. modder should know what he is doing |
23:16 |
kahrl |
does this one extra check really make any difference? |
23:16 |
RealBadAngel |
yes it does. it is GFX |
23:16 |
kahrl |
but only few nodes are mesh nodes |
23:17 |
RealBadAngel |
all nodeboxes are meshes |
23:17 |
RealBadAngel |
we dont have nodeboxes runtime anymore |
23:17 |
kahrl |
even if there are many, the branch should always be predicted as "not taken" on modern processors |
23:17 |
kahrl |
(assuming the cache is active) |
23:18 |
kahrl |
(or "taken" if the compiler reorders the code) |
23:18 |
RealBadAngel |
compilers are not magicians |
23:18 |
RealBadAngel |
if logic require too many steps its faulty |
23:19 |
kahrl |
have you done any actual measurements? |
23:19 |
RealBadAngel |
i could but what for? for sake of 3 lines of code being reordered? |
23:20 |
RealBadAngel |
thats minimal logic to solve the task |
23:20 |
kahrl |
meh, I don't care enough to continue this discussion |
23:21 |
kahrl |
fix the missing spaces after } and then push the code already |
23:22 |
RealBadAngel |
ok, testing now code without creating new mesh pointer |
23:23 |
RealBadAngel |
lol, i was placing some meshes around, put a sphere under my feet |
23:23 |
RealBadAngel |
now im stuck inside a sphere :) |
23:25 |
kahrl |
oh wait, about https://github.com/RealBadAngel/minetest/commit/b3b69809ab80032167f456e88609b48d9a9d766b#diff-c03ca828c6b8a7695f2cd7e52c316a3cR1735 |
23:25 |
kahrl |
wouldn't it be possible to eliminate this branch (yay!) if the conversion was always done |
23:25 |
|
zat joined #minetest-dev |
23:27 |
RealBadAngel |
conversion in nodedef is done only when caching is enabled |
23:27 |
RealBadAngel |
otherwise its converted runtime |
23:28 |
kahrl |
https://github.com/RealBadAngel/minetest/commit/b3b69809ab80032167f456e88609b48d9a9d766b#diff-70868aa6d6b96c0c1623c761500d23c4R884 |
23:28 |
kahrl |
set k = wm_to_6d[k] and access f->mesh_ptr[k] instead of f->mesh_ptr[j] |
23:28 |
kahrl |
err k = wm_to_6d[j] |
23:29 |
kahrl |
also it's kind of ugly to have the wm_to_6d table duplicated in nodedef.cpp and content_mapblock.cpp but meh |
23:30 |
RealBadAngel |
if wallmounted was compatible with axis dir i would use just j*4 |
23:30 |
RealBadAngel |
but it even upside down for 0 |
23:31 |
kahrl |
uhhhm wait |
23:31 |
RealBadAngel |
it could be just perfect if we could change wallmounted |
23:31 |
kahrl |
isn't this also a memory leak when it does f->mesh_ptr[0] = cloneMesh(f->mesh_ptr[0]) |
23:32 |
kahrl |
I mean, if you made the change I suggested |
23:32 |
kahrl |
ugh |
23:33 |
RealBadAngel |
mesh_ptr are all NULL before entering here |
23:33 |
kahrl |
except mesh_ptr[0] |
23:33 |
RealBadAngel |
yes |
23:35 |
kahrl |
well just push it as is |
23:36 |
kahrl |
(with the space after } change that I mentioned, of course) |
23:36 |
RealBadAngel |
sure |
23:36 |
kahrl |
and assuming the memleak fix worked |
23:36 |
RealBadAngel |
yes, but i do need casting there |
23:37 |
kahrl |
what for? |
23:37 |
RealBadAngel |
i need clone to work on IMesh |
23:37 |
RealBadAngel |
it will be needed for animated ones |
23:37 |
|
shadowzone joined #minetest-dev |
23:37 |
RealBadAngel |
so in case of static, casting is needed |
23:38 |
kahrl |
I don't get it |
23:38 |
RealBadAngel |
it doesnt do automatic conversions |
23:39 |
RealBadAngel |
i have to cast it |
23:39 |
kahrl |
in what line do you actually use anything from SMesh as opposed to IMesh? |
23:40 |
RealBadAngel |
all animated ones will require IMesh |
23:40 |
kahrl |
so why not use IMesh now? |
23:41 |
RealBadAngel |
im not sure but it could broke all collecting methods |
23:41 |
kahrl |
how? |
23:42 |
RealBadAngel |
wait a sec |
23:42 |
RealBadAngel |
im already using clone without casting in nodedef.cpp |
23:42 |
kahrl |
the contract of cloneMesh() states that the return value is an IMesh*. It doesn't guarantee that it's an SMesh*, so callers shouldn't rely on that |
23:42 |
RealBadAngel |
why it fails in content_mapblock? |
23:43 |
|
shadowzone joined #minetest-dev |
23:43 |
RealBadAngel |
/home/realbadangel/minetest-meshes3/src/content_mapblock.cpp:1751:49: error: invalid conversion from ‘irr::scene::IMesh*’ to ‘irr::scene::SMesh*’ [-fpermissive] |
23:43 |
RealBadAngel |
thats without casting |
23:43 |
kahrl |
even when you do scene::IMesh* mesh = cloneMesh(f.mesh_ptr[0])? |
23:44 |
RealBadAngel |
no, SMesh |
23:44 |
kahrl |
you have to change the type of the variable to IMesh* of course |
23:44 |
RealBadAngel |
note that mesh_ptr are of SMesh* |
23:44 |
RealBadAngel |
and in nodedef it compiles and works |
23:45 |
kahrl |
nope, they're IMesh* |
23:45 |
RealBadAngel |
*facepalm* |
23:46 |
RealBadAngel |
ok, no more questions :) |
23:55 |
RealBadAngel |
} else if (f.mesh_ptr[0]) |
23:55 |
RealBadAngel |
i think i dont need to check for the pointer here again |
23:56 |
RealBadAngel |
first if checks for all the possible pointers including 0, both with cache and without |
23:56 |
RealBadAngel |
so the "without cache" code is executed only for facedir != 0 |
23:57 |
kahrl |
where are you? |
23:57 |
RealBadAngel |
and pointers surely NULL |
23:57 |
kahrl |
nodedef.cpp:881? |
23:57 |
RealBadAngel |
https://github.com/minetest/minetest/pull/1778/files#diff-c03ca828c6b8a7695f2cd7e52c316a3cR1749 |
23:58 |
kahrl |
hmm |
23:59 |
kahrl |
what if all the mesh_ptrs are NULL? |
23:59 |
kahrl |
could that theoretically happen? |
23:59 |
RealBadAngel |
well, yes |