Minetest logo

IRC log for #minetest-dev, 2019-08-23

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
00:01 harrison joined #minetest-dev
00:10 Cornelia joined #minetest-dev
00:36 pauloue joined #minetest-dev
00:38 pauloue joined #minetest-dev
00:51 Cornelia joined #minetest-dev
01:02 Cornelia joined #minetest-dev
01:07 pauloue joined #minetest-dev
01:14 ANAND Approvals needed for simple bugfix PR #8342
01:14 ShadowBot https://github.com/minetest/minetest/issues/8342 -- Correctly set visibility of local player's attached objects by PiezoU005F
01:14 ANAND I've tested, and it works correctly.
01:31 Megaf joined #minetest-dev
01:40 Cornelia joined #minetest-dev
01:58 Cornelia joined #minetest-dev
03:58 behalebabo joined #minetest-dev
04:22 DeadMan2 joined #minetest-dev
04:27 atlas_ joined #minetest-dev
04:36 atlas_ joined #minetest-dev
04:37 atlas_ left #minetest-dev
05:53 Dead_Man joined #minetest-dev
05:59 Dead_Man Okay, is this working now?
05:59 Dead_Man I assume it is. Hello, all! Working on learning the rendering engine and pipeline so I can expand on it! First objective is shadows.
06:00 Lone-Star joined #minetest-dev
06:01 Dead_Man I'll be sure to ask here if I have any questions.
06:33 ANAND sfan5: Apparently, you'll have to prepend "fixes" to each individual issue number in https://github.com/minetest/minetest/pull/8832#issue-309269058, for all of them to be closed when the PR is merged.
06:39 ANAND #8342 requires a rebase, but the author seems to be quite inactive on Github
06:39 ShadowBot https://github.com/minetest/minetest/issues/8342 -- Correctly set visibility of local player's attached objects by PiezoU005F
06:40 ANAND Their last activity seems to be this very PR, which was created on Mar 8
07:05 fluxflux joined #minetest-dev
07:26 calcul0n joined #minetest-dev
08:23 Player-2 joined #minetest-dev
08:31 Lone-Star joined #minetest-dev
08:53 BuckarooBanzai ANAND: he quit mt a while ago :(
08:54 ssieb joined #minetest-dev
09:08 ANAND Oh no
09:15 calcul0n joined #minetest-dev
10:19 p_gimeno wow, 8046 (now #8342) was not yet merged? I thought it already was. I came up with a very similar fix in https://github.com/stujones11/wield3d/issues/2#issuecomment-459990306
10:19 ShadowBot https://github.com/minetest/minetest/issues/8342 -- Correctly set visibility of local player's attached objects by PiezoU005F
10:21 ssieb joined #minetest-dev
10:21 p_gimeno I can see the frustration of not getting a very simple, straightforward, clearly right bugfix sitting there for so many months
10:22 p_gimeno -not
10:27 Fixer joined #minetest-dev
10:29 sfan5 ANAND: please test #8342 again
10:29 ShadowBot https://github.com/minetest/minetest/issues/8342 -- Correctly set visibility of local player's attached objects by PiezoU005F
11:08 proller joined #minetest-dev
12:31 Cornelia joined #minetest-dev
12:52 Cornelia joined #minetest-dev
12:59 Megaf joined #minetest-dev
13:02 Wuzzy joined #minetest-dev
13:06 behalebabo joined #minetest-dev
13:37 Lia joined #minetest-dev
13:38 pmp-p i think there's something wrong here https://github.com/minetest/minetest/blob/master/src/client/renderingengine.cpp#L325  regarding issue #8841
13:38 ShadowBot https://github.com/minetest/minetest/issues/8841 -- rogue behaviour on mate-desktop (marco WM +compositing)
13:38 pmp-p metacity is complaining and it should not
13:39 pmp-p so there may be a problem MT side
13:48 pmp-p when building again GLES + x11 there's a small final linking problem since link.txt will miss a -lGL with the -lEGL
14:07 behalebabo joined #minetest-dev
14:13 TC01 joined #minetest-dev
14:42 Lone_Wolf joined #minetest-dev
14:51 ANAND sfan5: See https://github.com/minetest/minetest/pull/8342#issuecomment-524345134
14:51 ANAND Something seems to be wrong
14:53 ANAND Some entities are showing up properly while the others aren't
14:59 sfan5 the only difference I can see between the old and new code is that updateAttachments() is not called after m_is_visible was called
14:59 sfan5 s/called$/changed/
15:07 Amaz joined #minetest-dev
15:31 TC01 joined #minetest-dev
16:26 * rubenwardy considers adding meta data support to schematics
16:26 rubenwardy also, built-in support for schematic mapgen - pretty please?
16:32 rubenwardy also, support for baked in lighting for schematics would be nice
16:32 rubenwardy urgh
16:32 p_gimeno sfan5: it looks to me like m_attached_to_local is not up to date at that point, because it was moved to updateAttachments()
16:34 p_gimeno it was moved there in 4aa9a669cb184b77213e8df82eb20eda5aad9004
16:38 p_gimeno oh forget that, setAttachment calls updateAttachments
16:43 Krock joined #minetest-dev
16:49 p_gimeno funny, the patch works for me with wield3d but not with 3d_armor... ANAND could you confirm that?
16:51 Krock optimally all attachment-relevant variables should be changed in setAttachment() or updateAttachment()
16:51 Krock and if that's not possible, add a flag so it does not overwrite that the next time the attachments update
16:53 Krock rubenwardy: sounds good. save the entire mapnode, including metadata if requested
16:53 Krock thing is that metadata tends to make schematics huge due to lack of compression
16:54 p_gimeno actually, the patch works except the first time. Switching to another tool and back makes it show and then it keeps working.
16:54 p_gimeno (in 3d_armor)
16:59 p_gimeno ANAND: I don't know how to see the health gauge, is that a new version of 3d_armor or something?
16:59 Krock separate mod. gauges.
16:59 p_gimeno k
17:00 rubenwardy I suggest storing the metadata in a footer
17:00 rubenwardy And compressing either just the footer or the whole thing
17:00 rubenwardy Maybe we should also have xmts for compressed schematics
17:01 Krock so mts aren't forwars compatible?
17:01 Krock #8846 works. will merge after #8812 in 15 minutes
17:01 ShadowBot https://github.com/minetest/minetest/issues/8846 -- Fix default hand definition not using wieldhand.png by Wuzzy2
17:01 ShadowBot https://github.com/minetest/minetest/issues/8812 -- [NO SQUASH] Occlusion: Code cleanup and better checks using "light_propagates" by SmallJoker
17:07 p_gimeno ANAND: I can't reproduce the behaviour you report in https://github.com/minetest/minetest/pull/8342#issuecomment-524345134
17:10 p_gimeno the gauge looks displaced though as it goes through my body, but that's not related to that PR, it happens in master to
17:11 ssieb joined #minetest-dev
17:15 Krock I'm looking for the term to describe: auto inline_function = [=] (type *param) { .. }
17:16 Krock merging the prs..
17:17 Krock p_gimeno: because it wasn't made for 5.0 models
17:17 p_gimeno ah
17:18 sfan5 rubenwardy: what are you referring to?
17:28 sfan5 >check.Y = CLOSEST_EDGE(pos_camera, block_bounds, Y);
17:28 sfan5 Krock: is this alright with a macro for you?
17:29 Krock yes, optionally also with "check" since it's got to be the same axis
17:29 Krock but that's fine too
17:30 Krock well, not necessary to check against the mapblock center in what currently exists as occlusion culling shootlines
17:30 Krock but would be *technically* correct
17:36 sfan5 whoops i just spotted two errors in my pr
18:07 Krock for debugging it should be possible to draw lines like it's done for the pointed selectionbox border
18:07 Krock to see where the occlusion actually goes
18:08 lllI1I joined #minetest-dev
18:14 Fixer_ joined #minetest-dev
18:16 tomraceror joined #minetest-dev
18:29 behalebabo joined #minetest-dev
18:33 HDMI_STECKDOSE joined #minetest-dev
18:35 Krock I'd like to do some performance testing in #8838 before it's merged
18:35 ShadowBot https://github.com/minetest/minetest/issues/8838 -- Make Mapgen::spreadLight use a queue by DS-Minetest
18:36 Krock mainly comparing it with #8832
18:36 ShadowBot https://github.com/minetest/minetest/issues/8832 -- Mapgen: Run light spread algorithm iteratively by sfan5
18:36 sfan5 sure
18:37 HDMI_STECKDOSE joined #minetest-dev
18:37 sfan5 neither seem to change the performance much, so you need a specific reproducable scenario to test the exact values
18:38 tomraceror joined #minetest-dev
18:38 Krock well, dummy backend should be enough
18:42 lhofhansl joined #minetest-dev
18:42 lhofhansl hi all... Planning to merge #8838 soon.
18:42 ShadowBot https://github.com/minetest/minetest/issues/8838 -- Make Mapgen::spreadLight use a queue by DS-Minetest
18:43 sfan5 another thing is that the other PR queues an entire area worth of light spreading before doing actual spreading
18:43 sfan5 lhofhansl: Krock wanted to do some performance testing first
18:43 lhofhansl Ok... I'll hold off.
18:43 lhofhansl BTW. testing #8812 / #8844 now.
18:43 ShadowBot https://github.com/minetest/minetest/issues/8812 -- [NO SQUASH] Occlusion: Code cleanup and better checks using "light_propagates" by SmallJoker
18:43 ShadowBot https://github.com/minetest/minetest/issues/8844 -- Improve occlusion culling by sfan5
18:44 sfan5 nice, thanks
18:45 Krock lhofhansl: fine, go ahead. sfan5's method lost by far
18:45 Krock https://pastebin.com/raw/qSFa2PML
18:46 sfan5 oh it's that much slower
18:46 lhofhansl On that. In the past I had experimented improving the accuracy by sending more rays (8 more, 1/2 between the block's center and the corner). Perhaps something to consider again.
18:46 sfan5 it it the memory allocations? 🤔
18:46 Krock yeah. I really wonder what's causing that
18:46 Krock well, there must be less insertions in DS' version
18:46 Krock the queue is shorter, or no duplicates
18:46 lhofhansl Cool. Do you normally merge via Github, or manually apply the patch? (I seen both recommended here)
18:47 sfan5 usually github these days
18:47 lhofhansl Aye. Will do. I think DS' version is faster because it avoids recalculations.
18:48 lhofhansl (with high light levels)
18:48 Krock that or queue.pop_front(); is expensive
18:48 sfan5 std::queue just wraps std::deque and calls pop_front() so that would be identical
18:49 Krock indeed, just seen that too
18:49 Krock pre-allocating the queue memory would surely help here
18:50 sfan5 it would also be quite funny to have a container called "queue" where pop/push aren't O(1)
18:50 Krock since it's probably deallocating in pop() and allocating in emplace() a few times
18:51 Krock unless there's some threshold internally to indicate the upper limit for bulk-allocate and lower limit for bulk-deallocate
18:51 GreenDimond joined #minetest-dev
18:53 Krock so why not a dequeue directly?
18:53 Krock testing that
18:54 lhofhansl Alright. Won't merge for now.
19:00 lhofhansl So #8812... The straight line special case is pretty awesome. I do find, though, that there are some new scenarios when you look just sideways along a straight tunnel that more blocks are occluded when they should not.
19:00 ShadowBot https://github.com/minetest/minetest/issues/8812 -- [NO SQUASH] Occlusion: Code cleanup and better checks using "light_propagates" by SmallJoker
19:00 HDMI_STECKDOSE joined #minetest-dev
19:00 lhofhansl Perhaps we should have a version first that leaves everything functionally unchanged, but adds the straight line special case...?
19:01 sfan5 are you referring to 8844, since 8812 is already merged?
19:01 lhofhansl I tested them together :)
19:01 lhofhansl Perhaps too late, and we fix the issues as we go.
19:02 lhofhansl Lemme pull and test with 8812 alone.
19:03 sfan5 the occlusion behaviour will most certainly be worse
19:04 lhofhansl As in it will incorrectly cull more blocks?
19:04 sfan5 yes
19:05 lhofhansl Hmmm... Why's that a good thing?
19:07 lhofhansl (It is indeed much worse in the scenario I tested)
19:11 paramat joined #minetest-dev
19:17 behalebabo joined #minetest-dev
19:18 lhofhansl Apologies for only really having time to look now... But this is way-way worse (at least without 8844). Could we not have done the refactoring without functional changes?
19:19 Krock testing with ringbuffer. deque turned out to be even worse
19:20 Krock or comparable, if done right
19:20 sfan5 lhofhansl: yes it could have been done, but we kinda wanted to anyway
19:21 sfan5 also I am well aware of the "way-way-worse" bit, I planned 8844 to be merged to bring occlusion culling much closer to how it performed before
19:21 lhofhansl I propose we undo that change. Commented to that extend on the PR.
19:23 Krock even a C-style static ringbuffer takes double the time?!? there must be some GCC optimizations that favour argument-passed data
19:24 sfan5 maybe "occlusion check stops 27 nodes before target block" should be made a feature
19:25 lhofhansl If it improves the visual... :)  The old code was more precise at close ranges and gets more coarse at a distance. I get the desire to cull more, but not if it does not look right.
19:27 sfan5 "more precise", well it just didn't cull anything that was close
19:29 sfan5 I do think even if the occlusion behavior is returned to how it was before that 8844 would make an useful addition
19:29 Krock https://pastebin.com/raw/qSFa2PML updated Ctrl + F5
19:30 Krock weird stuff. there's now no longer memory allocations in the ringbuffer
19:31 lhofhansl The server is also culling (i.e. not sending culled blocks). The client number is not necessarily the full story.
19:31 lhofhansl I'm all for 8844, it's an awesome addition!
19:32 lhofhansl DS' version is magic :)(
19:32 Krock I seriously don't get why it's faster
19:33 lhofhansl (Now I do not get it anymore either. Is it still correct?)
19:34 proller joined #minetest-dev
19:34 lhofhansl sfan5: Let's add another PR to bring the functionality back to what it was. Then merge 8844. Then look for further ways to improve this (8812 makes that easier).
19:41 sfan5 alright
19:43 lhofhansl Unfortunately with the tracing we do there's always the trade-off between performance and accuracy... :(
19:43 lhofhansl Minecraft does this: https://tomcc.github.io/2014/08/31/visibility-1.html
19:43 lhofhansl (Not that we should do that, too, but it's interesting)
19:44 Krock Q: what spreads the light client-side?
19:45 Krock not just that. if torches are placed after mapgen, it's not spreadLight that is called
19:45 Krock * lightSpread
19:46 lhofhansl It's not? It has to be done server side, no? Or other clients won't see it...
19:47 Krock voxelalgorithms spread_light
19:49 Krock that's basically triple the code of mapgen, except that mapgen has its own implementation
19:49 sfan5 oof
19:49 Krock guys you're optimizing at the wrong place I guess
19:50 sfan5 well the main idea was to fix segfaults due to stack overflow, the optimization idea came later
19:51 lhofhansl Still nicer code and faster. I'd still say let's merge.
19:51 sfan5 of course
19:51 GreenDimond left #minetest-dev
19:52 lhofhansl DS' improved it even further :)
19:52 Krock add a comment referencing to voxelalgorithms
19:52 Krock it shall not be forgotten
19:52 lhofhansl +1
19:53 lhofhansl Gotta head out in a bit. If it's hard to fix, let's revert 8812 and regroup. I very much like the refactoring of 8812 and 8844 is definitely a vast improvement for tunnels!
19:54 sfan5 lhofhansl, Krock: https://github.com/minetest/minetest/pull/8850
19:55 lhofhansl Looking...
19:56 lhofhansl Testing...
20:00 lhofhansl Cool. I guess we can leave the step = 1.2 and stepFac = 1.05 (it was 1.0 and 1.1 resp, before). I can't see much a difference to before.
20:00 Krock so if (distance < BLOCK_DIAGONAL) isn't enough?
20:00 Krock stepfac has to be small. 1.1 is way too high
20:01 lhofhansl Depends on what the goal is. Close blocks fill more of the view-area so being precise there is important, as we go into the distance the culling becomes less and less effective anyway,
20:02 Krock oh I see. step might be higher than BLOCK_DIAGONAL so that it's never reached
20:02 Krock s/step/offset/
20:02 Krock s/.*//
20:02 lhofhansl I.e. a smaller starting step and then a larger stepFac. But... as I said I didn't see a difference in behavior is my tests.
20:03 Krock a smaller step (towards 1.0) results in more node lookups, hence either better occlusion checked blocks or more false-positibes
20:03 Krock *positives
20:04 lhofhansl (btw. I had a PR decreasing stepFac before, for some reason we decide to revert it, but I forgot why. I agree smaller is more accurate, obviously)
20:04 lhofhansl 8850 looks good to me.
20:07 Krock lhofhansl: stepfac = 1.0f  and master/HEAD should actually result in the same thing
20:07 lhofhansl You mean step = 1.0f. stepfac should always be > 1.0.
20:08 lhofhansl (Unless we do not want to decrease accuracy with distance)
20:08 Krock step = 2, stepfac = 1
20:08 Krock former * BS ofc
20:10 lhofhansl As an alternative we could *only* check close blocks and just stop after a certain radius. Oh well... That's for other PRs.
20:12 lllI1I joined #minetest-dev
20:14 sfan5 is #8838 ready for merge then?
20:14 ShadowBot https://github.com/minetest/minetest/issues/8838 -- Make Mapgen::spreadLight use a queue by DS-Minetest
20:15 lhofhansl +1
20:15 Krock yes
20:16 sfan5 merging it then
20:18 sfan5 done that
20:18 sfan5 8844 is also updated (depends on 8850 being merged first)
20:20 sfan5 Krock: any outstanding issues with either of those? ^
20:22 Krock former surely needs testing, regarding the latter I'm still don't know what's going wrong
20:22 lhofhansl In 8838 we do not have the comment pointing to voxelalgorithms...
20:23 Krock mhm well so
20:24 sfan5 Krock: i think what is going wrong there are the (up to) 11 nodes that are closer than BLOCK_DIAGONAL but not within block bounds
20:24 sfan5 those make the difference in culling here
20:25 sfan5 well it's up to 27 actually depending on which corner you check, up to 19 for the center
20:25 Krock that ^
20:26 Krock so in fact it can skip an entire mapblock
20:31 Krock oh wait I got it
20:32 lhofhansl No objections to 8850 and 8844 from me.
20:33 Krock in a corridor it's easier to hit the wall in between the pos_target and pos_camera. ignoring up to 27 nodes allows far away mapblocks to be drawn where otherwise would be solid nodes in the last few meters
20:34 Krock the IMO sane solution for this would be to alter "count" depending on distance
20:35 Krock because nearby nodes have a higher occlusion culling factor than far away ones
20:35 lhofhansl That's a good idea. The fact count itself is just heuristic and get it mostly right. I tried to remove it before and improve accuracy instead and it turned out it was too hard to tune the rest accordingly.
20:39 lhofhansl Or have some other way to ensure that each node is guaranteed to be consulted at most once.
20:40 lhofhansl Alright... Gotta head. I'm all fine with merging 8850 and 8844 (added my One Approval label).
20:40 lhofhansl left #minetest-dev
20:41 Krock uh well, I'd prefer to see "count" to be changed rather than adding this no-check-hack
20:46 sfan5 the question is how hard is it to get that adjustment exactly right
20:48 Krock worst case is known. 1x2 corridor, between mapblock center and mapblock edge
20:48 sfan5 I don't disagree, but it would be better to make it works equally good before (or even better with 8844) and then when someone takes the time to fix it properly, merge it then
20:49 sfan5 good as before*
20:50 Krock +1 if you want to restore the last-good state. +2 if you'd have the time to fix "count"
20:50 Krock or me.. would need to test that tomorrow if so
20:51 sfan5 I think 8844 improves occlusion culling in a more user visible way than fixing "count" to be somehow more accurate, so I didn't plan on doing the latter
21:04 Krock count += MYMIN(1.0f, 20.0f / offset);  works for me
21:43 paramat joined #minetest-dev
22:09 behalebabo joined #minetest-dev
22:43 lhofhansl joined #minetest-dev
22:46 lhofhansl Back... Let's restore the current behavior (more or less) with 8850. Then we can improve stuff (such as 8844, and Krock
22:46 lhofhansl 's count fix)
22:57 lhofhansl left #minetest-dev
23:19 Cornelia joined #minetest-dev
23:24 rom1504 joined #minetest-dev
23:25 sfan5 agree, will be merging #8850 in 15 minutes
23:25 ShadowBot https://github.com/minetest/minetest/issues/8850 -- Restore approximate occlusion check by sfan5
23:27 paramat joined #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext