Time Nick Message 04:48 erlehmann josiah_wi could you tell me how exactly the image rendering tests are fragile? 12:45 MTDiscord erlehmann, if any one of a number of components changes by a single pixel, the test will break. It is reasonable to think that we could make an intentional change that slightly offsets the image, and to fix the test will require taking a new screenshot. And that's besides the test mentioning itself that there are unhandled cases that break it. 12:51 erlehmann josiah_wi if you intentionally break the rendering, you'll know. however, all of the bad things that happened so far to rendering would have been caught by reference image checks. 12:52 erlehmann josiah_wi i mean i already found that some rendering tests break with LIBGL_ALWAYS_SOFTWARE=1 which is a bad thing. by not having the tests, you'd just not notice. 12:53 erlehmann josiah_wi so how else are you going to notice when some “optimization” breaks stuff? 12:53 erlehmann you noticed yourself that one test you deleted was flaky bc the implementation was just wrong 12:54 erlehmann deleting the test surely will not improve the implementation 12:54 erlehmann josiah_wi are you maybe trying to only have passing tests that won't break so that it will be merged? 12:55 erlehmann or is there something fundamentally wrong with expecting that a rendering matches a reference picture? 12:55 MTDiscord I would say that there is something wrong with the latter 12:55 erlehmann like, is irrlicht non-deterministic by design? i mean, it is in practice, lots of UB 12:56 MTDiscord Anything related to graphics card used to involve lots of vendor-specific behavior 12:56 erlehmann luatic what do you mean with “the latter”?? 12:57 erlehmann well i don't think it matters which tests are included at first, as long as there are some 12:57 erlehmann i just think it is a bad idea to say “the tests might break bc someone might change the software” 12:58 erlehmann from my experince, if someone breaks a test case, they usually are happy that it was caught. 12:58 erlehmann experience 12:58 MTDiscord Indeed. Changing the software should require updating the tests, unless it's merely fix. 12:58 MTDiscord a fix* 12:59 erlehmann IMO a fix should also require a regression test, but last time i suggested that, sfan5 taunted me with “pls write a test case for this commit that fixes a bug that you have never heard of” 12:59 erlehmann so i'll keep to my own turf there 13:02 erlehmann luatic i have seen the reaction “just disable/delete/skip the tests” when someone changed something at work too, but in 100% of the cases the person suggesting or doing that was either inexperienced or the type of arrogant dev that ends up not working well with others (bc their features are full of bugs) 13:02 erlehmann if the tests are reasonable, almost everyone will update them – or more likely, fix their bugs. 13:03 MTDiscord "or is there something fundamentally wrong with expecting that a rendering matches a reference picture?" Yes, it is. This is one area where results will vary with hardware and drivers, to the extent that professional devs do not typically attempt to validate these aspects with automated testing and deliberately standardize the graphics setups of their workstations across an entire game production, down to forcing the exact same 13:04 MTDiscord driver version 13:04 MTDiscord If you want to test only software rendering, you may get better results. But then those results won't correspond to any real-life usecase 13:05 MTDiscord I wonder whether we could use something like an image similarity metric? 13:21 MTDiscord haha gamma correction goes brrrr 13:29 MTDiscord instead of arguing over driver specifics why not exclusively target OGL 2.1 13:30 MTDiscord there is a way to get 4.3 on macOS with OpenGL core profiles 13:32 pgimeno problem with tests that detect bugs in a project like this is that they need people who are willing to fix the bugs, otherwise it's meaningless to have the tests because they will fail every time 13:33 erlehmann pgimeno it is not meaningless to have tests that do not fail though 13:33 pgimeno yeah, but once one test breaks, you have the first case 13:34 MTDiscord If a broken test indicates a bug in your code, it will be blocked from getting merged. Not because the test is failing, but because your code is wrong. 13:34 pgimeno not always 13:34 pgimeno it may fix more than it breaks 13:34 pgimeno and there may be no obvious alternative that doesn't involve a lot of work 13:35 erlehmann so far i have only seen accidental breakage from most ppl 13:35 MTDiscord In other words, we knowingly let people PR regressions? 13:35 MTDiscord Because "it fixes more than it breaks"? 13:35 MTDiscord I'd like to hear that from a couple core devs. 13:36 pgimeno yeah, it's happened a number of times 13:36 erlehmann pgimeno so you want rather ppl to unknowingly approve regressions? 13:36 MTDiscord Ouch. Well, the good news is, the tests I intend to PR will only break if someone messes up fundamentally, so managing to fail them at all should be pretty difficult. 13:37 sfan5 you know, all this discussion about irrlicht is not proportional to its importance 13:37 pgimeno showing help as a formspec broke text output, and it was merged anyway. I was willing to fix it and submitted a fix, otherwise it might still be unfixed. 13:37 MTDiscord sfan5, thank you. 13:37 sfan5 the renderer will eventually be rewritten inside minetest, the irrlicht code still stay around almost unmodified until one day it's removed 13:37 erlehmann sfan5 well that part will need tests too 13:38 erlehmann pgimeno tests are a mechanism. what policy you make of that is something different. 13:38 sfan5 but it won't need the tests are are ported/written now, it's all the in the future 13:39 erlehmann well, the irrlicht stuff has already been unknowingly broken 13:39 sfan5 in what way? 13:39 erlehmann when stuff was removed 13:39 erlehmann like you removing the xml stuff without noticing you should remove the fonts as well 13:39 erlehmann if you want it gone completely 13:39 sfan5 no 13:40 sfan5 I was fully aware that it breaks xml bitmap fonts 13:40 sfan5 but I'm glad you are here to tell me that I didn't know 13:40 MTDiscord erlehmann, I don't think your frequent discussions of this stuff here are necessarily helping my cause. Nor are other people particularly interested in it. 13:40 erlehmann sfan5 i apologize 13:41 erlehmann josiah_wi ok, i'll do sth else. pls ping me when your stuff gets merged so i can write the regression tests. or ping me if you need some more crashes, i'll find some. 13:41 MTDiscord Will do. 15:02 MTDiscord Ah yes, we would need JSON support in mtirrlicht in order to support .gltf models just a heads up 15:02 MTDiscord XML is handy, but not needed 15:05 sfan5 a gltf loader should reside in Minetest where we already have jsoncpp 15:16 MTDiscord But don't we need to load it into itrlicht like we do for b3d and obj? 15:16 MTDiscord Or does irrlicht expose a function to let us transfer "raw irrlicht model data" somehow? 15:24 sfan5 of course 15:46 MTDiscord Irrlicht exposes an API for projects to register external image loaders. 15:46 MTDiscord I actually wondered whether we should use that for WebP. 15:47 MTDiscord I don't know whether it does the same for model loaders. 15:56 sfan5 it does 16:49 MTDiscord Cool, okay so webp and all these loaders can be in minetest and not irrlicht which would help if we ever upgraded 19:41 MTDiscord I guess I better move my FindWebP script over to a Minetest branch sometime.