Time Nick Message 04:56 hecks I've noticed that shadow mapping disables greedy meshing, why? 06:05 MTDiscord Yes, because that meshing creates triangles too big with non-connected vertices. That plus small precision issues between those triangles makes glitches on the render. One edge of the triangle gets a distortion, but the same edge for other face get much more distortion. 06:05 MTDiscord So you get gaps between faces 06:38 hecks You get gaps between faces because you're using normal offset bias on meshes with hard edges, lol 06:39 hecks the mtg "sam" player model has those gaps too 06:46 hecks oh but there could be t-junction cracks 06:46 hecks which we see even in "normal" rendering so maybe greedy just be disabled in general 06:47 hecks *can just be disabled 06:48 hecks and we ought to look for bandwidth reductions elsewhere like culling unseen nodebox faces 07:44 MTDiscord Not even close. At the time I impl that, I didn't even ear about normal offset bias. 07:48 MTDiscord with the force_no_tiling to false: 07:48 MTDiscord https://cdn.discordapp.com/attachments/747163566800633906/860426564579754014/unknown.png 07:48 MTDiscord its because the distortion fn in the shadow render 07:56 MTDiscord and in case you are wondering why, this is how the tiling is done: 07:56 MTDiscord https://cdn.discordapp.com/attachments/747163566800633906/860428683076042752/unknown.png 07:57 MTDiscord so, the distortion fn creates much more distortion for top layer and it creates the gap. 12:09 VanessaE Liso: but you have to admit: the distorted (force_no_tiling) version has no "fresnel lens" effect either... 12:30 MTDiscord Yes, you are right. But it's the only option we have to create shadows for long portions of the world without CSM. If we could reduce the drawcalls under 1k we could be able to use CSM and no distortion at all in less than a weekend 12:32 MTDiscord At least it's the only solution i know, I mean 12:39 VanessaE consider hecks' sentiment 12:40 VanessaE go ahead and do it with all the drawcalls you need, but spread them out over as many frames as it takes to keep the added jitter under some limit 12:40 VanessaE if only to get a sense of its performance 12:41 VanessaE maybe it won't be as bad as you imagine? 12:43 MTDiscord What does #11405 require still to get merged? 12:43 ShadowBot https://github.com/minetest/minetest/issues/11405 -- Add API for mods to hook liquid transformation events by Warr1024 12:44 VanessaE I mean, if it takes let's say 10000 calls to fill-out the shadows and your target is to refresh that every 0.2s... if you're getting say 40 fps, then you'd be doing about 1250 extra drawcalls per frame 12:45 VanessaE (10k is just a hopefully outlandish worst-case example, idk how many calls you're *actually* looking at) 12:46 MTDiscord ok, let me try 12:47 VanessaE I'm just guessing here Liso 12:47 VanessaE Warr1024: a sacrifice maybe? :P) 12:47 VanessaE :P 12:49 MTDiscord no, it's ok. but you made a good point. I assumed CSM slowness without the lasts changes like better frustum, (or adaptive one that x2048 is working on). I must test it 12:49 MTDiscord It turned out to be a pretty trivial change, much less complex than I expected. I've had a handful of other core devs weigh in, and they've asked questions which have been addressed, but nobody has officially commented, and it's stuck in limbo. 12:49 MTDiscord Is there like some "standard waiting period" for rebuttals before something would get merged? 12:50 MTDiscord I don't think that process would work anyway because by now people are probably just assuming nothing will get merged anyway and not watching for stuff to raise issues with. 12:55 MTDiscord Am I supposed to call someone out specifically to do a review or something? 12:56 VanessaE not really 12:56 VanessaE you just have to poke whoever shows up 12:56 VanessaE or more generally just remind the channel 12:57 MTDiscord poke until puke 12:58 MTDiscord why do you pass a list of positions instead of calling the callback per position? 12:58 MTDiscord performance? 12:58 MTDiscord It seems like before, I was able to get 2 approvals for merges within a couple of hours if I poked the channel in IRC. Now I only need 1 more approval, but I've spammed this PR here over the course of about 3 days and gotten barely any response. Has stuff changed that much, or was I just really insanely lucky with my earlier PRs? 13:17 celeron55 well you could literally hilight people and tell them to do what you want, but on the other hand sometimes people are just busy doing other things (which is the case for me at the moment, or actually for the following month probably) 13:19 celeron55 usually linking a PR here and asking for whatever makes sense 13:19 celeron55 like, generally asking 13:19 celeron55 like you already did 13:26 sfan5 I could review the PR but I've not fully made up my mind yet on whether I like the proposed API 13:27 celeron55 to me it's fine, i guess i'll review 13:27 celeron55 it's the most minimal implementation possible 13:34 VanessaE wait..wait...c55 is reviewing a PR? ;) 13:37 MTDiscord I naively copied and pasted the code in from another branch for a dead pre-PR, but it didn't work, so I copied the guts of pushnode. If I can find where I can import an existing pushnode from then I'll use that. 13:38 celeron55 well just use grep or something, it's there 13:38 MTDiscord I assumed people could be busy and hence why I refrained from poking anyone, including the people who had weighed in before, but I guess we might have an "if it's everyone's responsibility then it's nobody's responsibility" issue. 13:38 celeron55 no it's literally nobody's responsibility to begin with 13:39 celeron55 we don't need to go from the everyone route to get to that one 8) 13:39 celeron55 i guess the amount of fame people get as compensation isn't enough 13:40 MTDiscord As for the proposed API, it's a baseline MVP essentially, and we COULD debate about what it should or should not have, but the debate is only productive if we're going to converge toward an answer. That sounds like the kind of thing that tends to kill PRs by bikeshedding. 13:40 celeron55 there just isn't that much to give when each release has tens of PRs included 13:40 MTDiscord If we have a Benevloent Dictator then we have a tiebreaker in all debates, but I don't know how much Benevolent Dictator we'll have available at any time ... :-/ 13:41 celeron55 well you don't need to wait forever for API suggestions 13:41 celeron55 a week from the creation of such PR should be enough for someone to suggest a better API 13:43 celeron55 if they don't, then the API is fine, and of course the APi can be changed as long as no release has been made 13:43 MTDiscord If we have a standard like 1 week for design changes then I think that could work. 13:44 celeron55 lua API changes are one of the most lightweight changes possible, the only thing they break is mods, compared to file or network format changes that break saved worlds or client-server compatibility 13:47 MTDiscord It seems like there should be some knowable process of keeping things moving forward, whether that's toward a merge, or a rejection, or through a bunch of reviews. Having the fire-and-forget stuff just stuck in limbo seems fine but I'd like to know that I'm doing the right thing to keep something that has an active champion for it moving forward. What I'm doing so far seems to be a process but I don't know if it's a preferable 13:47 MTDiscord one. 13:47 celeron55 and if let's say 3 or 4 core devs say it's good then no need to wait at all. but of course you should aim for getting the best result out of the process 13:48 MTDiscord I mean I sort of assumed that to merge anything I needed to have at least a second formal approval on the thread for the PR itself... 13:52 celeron55 that is true 13:54 MTDiscord So I guess if I resolve any code review problems raised, and nobody can make a concrete objection to the API as designed, then after it's a week old, I should start poking again? 13:54 celeron55 at one point in time it wasn't so, core devs were assigned subsystems that they could change on their own will 13:54 celeron55 it resulted in quite a lot of questionable quality 13:55 MTDiscord Yeah, it seems more like we would want to start from the other direction, e.g. have core devs who are assigned "must comment" items or something. 13:56 celeron55 you should note that my review is an approval given the code changes i commented are possible and are made 13:56 MTDiscord Ah, great, thanks. I'm looking for where I can find pushnode. 13:56 MTDiscord I haven't done C++ on the level of "remembering how to navigate the include system" since like 1999 or something, so... 14:07 MTDiscord Come on, the include system isn't that complicated. 14:09 sfan5 also a super minor thing, you probably copied std::vector > from somewhere else but it should be std::vector> 14:10 sfan5 spacing the >'s is a relict from when C++ compilers would misinterpret consecutive sharp braces 14:13 MTDiscord I could swear I looked at that, thought "oh, that looks weird, maybe I should fix that", tried to compile, it didn't work, then put the space back and thought "oh, I guess that's why that was there :-/" 14:13 MTDiscord I can give it another try, maybe it was a different error that crept in and got confounded. 14:20 MTDiscord Yeah, removing the space worked fine, dunno why that was messing with me before. 14:30 pgimeno hm, it's a scanner thing, previous to the parsing phase, it's weird that it's accepted 14:31 pgimeno maybe they split the ">>" token (used for right shifts) into two ">" and left up recognition of those to the parser 14:32 pgimeno I'm slightly off-topic though 14:40 MTDiscord In my case it was probably a case of forgetting the "only the first compile error is valid, ignore all the rest" rule. 14:44 MTDiscord I made all the requested changes, built and tested, and marked the conversations as resolved. It still says "changes requested" in GH though ... is that normal, like, to hold off on the full approval until we've been through the design approval waiting period? Or am I githubbing wrong? 14:47 sfan5 that status only goes away if the reviewing user updates their review 14:49 MTDiscord Okay. I have an option to request a re-review. Am I supposed to do that, or should I wait until the PR has been a week old so that c55 doesn't get bothered about it until it's actually actionable? 14:49 sfan5 you should note that my review is an approval given the code changes i commented are possible and are made 14:50 MTDiscord So I guess after the week is over, it can just be merged even if there's red showing up on GH? 14:51 celeron55 we have our own processes, github is just a tool we try to use as well as it happens to fit 14:52 MTDiscord Okay, I've been sort of assuming that getting all-green to show up across the board was like a first gate that any work has to pass. 14:52 celeron55 github could show a 100% red page and tell you to shut down your computer and stop programming ever again and you could still merge if you have two approvals 14:54 MTDiscord Haha, okay, I mean, I sort of assumed that 2 approvals was what I needed, nothing more, nothing less, but it's just weird to think that I COULD get 2 approvals without passing the automated check gates :-D 14:58 celeron55 wait what, one can request a review from oneself 15:02 MTDiscord I suppose that makes some kind of sense. I do that as an internal process, e.g. when I check over the diffs before making a commit. I suppose if I were juggling enough crap a the same time and wanted to make a "just so I don't lose my work" commit then I could use something like that to remind myself I hadn't actually reviewed my own changes yet. 15:03 MTDiscord Seems like a weird process to have formal tools for, but I guess maybe it was easier for GH to allow it than to prevent it. 15:07 sfan5 you *can* enable something to prevent pushing commits unless they are part of an N-times approved PR with all checks green 15:07 sfan5 but we don't have that, fortunately 15:09 MTDiscord I guess it depends on how much you want to leverage the tools GH provides you to do things. Relying on them more can streamline processes and make things less ambiguous, but it would also create lock-in, make it harder to move the project elsewhere if needed, and GH would have less incentive to deal honestly with you. 15:22 celeron55 it's also about the past: historically, for most of the existence of MT, github didn't have that review system 15:23 celeron55 but it's true also that most competitors also still don't have it, so requiring it would create lock-in, which given that many don't like github's ownership would be a bad play 15:26 MTDiscord I am seeking approvals for #11287. I have acted on all the requested changes. 15:26 ShadowBot https://github.com/minetest/minetest/issues/11287 -- Take advantage of IrrlichtMt target by JosiahWI 15:30 MTDiscord Oof, that scrolling code block at the top. I assume the important part about what that PR is supposed to mean is in there, but it's hard to read it that way... 15:35 MTDiscord Yes, you've identified the most serious issue with the PR haha. How can I get fix that? 15:43 MTDiscord I added manual line breaks. I don't know why GitHub thought putting long paragraphs in a scrolling code block was good. 15:45 MTDiscord Ohh interesting, it was my indentation. Fixed. Sorry for the inconvenience to those who had to suffer reading the code block. 15:46 sfan5 does anyone want to test that PR with cmake 3.5? 15:46 sfan5 (our minimum supported version) 15:48 MTDiscord Why don't we use version 3.5 on one of the CI builds btw? Is it not easy to choose an older version? 15:50 sfan5 github runners come with a very recent cmake version installed and I don't think 3.5 specifically is in the repos 16:03 sfan5 guess I'll test it locally then 16:04 sfan5 in a container, because I won't be installing a debian 9 system manually 16:12 MTDiscord Containers are nice ... it'd be nice to have some like docker configs somewhere (maybe even in-tree?) for edge test cases like this so that you can enlist pretty much anybody to run these sorts of things for you. 16:12 MTDiscord i.e. if you're going to do the work to set it up anyway, if you can script it out without going too much further out of your way, it could become an automated regression test of sorts. 17:11 sfan5 "You have called ADD_LIBRARY for library IrrlichtMt without any source files. This typically indicates a problem with your CMakeLists.txt file 17:11 sfan5 " does it really? 17:14 sfan5 it built fine at least 17:15 sfan5 @josiah_wi did you see the comment on one of the older PR conversations? 18:19 MTDiscord The one about removing FindIrrlicht.cmake? 18:20 sfan5 yes 18:21 MTDiscord Was the add_library warning about the imported target? If it built normally then it's probably CMake being not-optimally-smart. 18:22 MTDiscord That would be strange though. CMake should know very well at all versions since imported targets were added that imported targets have no source files. 18:23 sfan5 I wouldn't worry about warnings cmake 3.7 throws 18:29 MTDiscord Unless it's a "missing project() call warning" or something, but normally if you get that there is very definitely a problem with your CMakeLists.txt file. 18:43 sfan5 -set(VERSION_MINOR 5) 18:43 sfan5 +set(VERSION_MINOR 3) 18:43 sfan5 uh wat 18:55 MTDiscord Oh no. sweats hard 18:55 MTDiscord I forgot to undo that change. One sec. 18:56 MTDiscord I was trying to connect to a server that thought I was hacking and I tried changing the version and removing the commit tag from it. 18:56 MTDiscord Apparently I forgot to switch branches. 18:57 sfan5 and did it work? 18:58 MTDiscord No, it must have been a protocol mismatch. I should have changed it back right away anyway, but it was late and I thought I had the habit to check the diff before committing; apparently not. 19:45 ShadowNinja I loaded up Minetest in RenderDoc and there are two obvious issues. The first is draw calls: a single frame is ~2000 draw calls. Each item in the hotbar alone is about 6 draw calls. The other issue is occlusion culling: most of the first 300 draw calls are for caves that are occluded by the surface. 19:50 sfan5 each item used to be 400 drawcalls before I fixed that ;) 19:50 ShadowNinja If anyone else wants to try it out here's what I had to do: 1) Enable OGLES2 in Irrlicht by editing include/IrrCompileConfig.h to set _IRR_COMPILE_WITH_OGLES2_ 2) Clean rebuild of Irrlicht (incremental build may not pick up the change because of the way the CMake script checks the flag) 3) Build Minetest with -DENABLE_GLES 4) set video_driver = ogles2 5) symlink client/shaders/Irrlicht to /media/Shaders 19:53 ShadowNinja You have to use OpenGL ES 2 because RenderDoc only supports modern Open GL contexts (3.2+ core) and ES 2.0+. 19:54 sfan5 it is possible to make IrrlichtMt use a core context btw but renderdoc will still choke on all the legacy calls 19:59 VanessaE ShadowNinja: 2000? now try one of the "acid" tests e.g. spawn area on one of my Dreambuilder servers :) 20:00 VanessaE (one assumes it varies by the amount of visible content) 20:03 MTDiscord sfan5: macOS uses core contexts over "latest" openGL as GL behaviour may have minor changes between versions and is preferred 20:05 sfan5 ohai 20:05 sfan5 when did you last build IrrlichtMt on macOS? 20:09 ShadowNinja VanessaE: About 14000 drawcalls 20:15 VanessaE eeeek :) 20:25 ShadowNinja LOL, even with that many drawcalls a started getting flooded with irrlicht errors for doing too much in a single draw call on your server: "Could not draw triangles, too many primitives(79502), maximum is 65535." 20:25 VanessaE heh 20:26 sfan5 that wouldn't be too much if irrlicht allowed non-indexed rendering 20:26 VanessaE what, did someone build a house out of pool tables? :) 20:26 VanessaE or one of the 3d printers :) 20:26 VanessaE (those are the highest-poly models I can think of) 20:33 MTDiscord You are right ShadowNinja, that's the main issue right now 21:11 MTDiscord why does irrlicht use shorts for indexing instead of ints? 21:19 sfan5 I can't find out if opengl supported that but for gles it's an optional extension 21:19 sfan5 it also wastes memory bandwidth 21:20 sfan5 s/opengl/opengl 1/ 21:20 sfan5 (is mandatory for gl2 upwards) 21:23 MTDiscord sfan5: works fine as of head 21:26 sfan5 alright ty 21:31 MTDiscord apparently getting macOS into OpenGL 4.3 mode is relatively straight forwards 21:32 MTDiscord well, 4.1 21:33 MTDiscord (but that's still lightyears ahead of 2.1) 22:21 sfan5 rubenwardy: looks like you forgot to update the link on the website after making the android build