Thing rendering by touched sectors

[Enjay]

Enjay, yes.
But you can also use RenderRadius if you don't want your physical collision box to be large.

Excellent and even more excellent. Thank you for this and for Graf merging it.

[ZZYZX]

https://github.com/coelckers/gzdoom/pull/129

Also forward_list is probably just as fast as Boom's implementation.
I believe the only reason why it's using the manual approach, is that most killough/phares code is labeled with the date on which C++ wasn't really good yet (98-99).
Since it's generally a rule of a thumb that library function from an old (20+ years) library is probably more optimized than the one you write manually.
Besides, now it takes 50 lines less. Otherwise I'd have to also write the code that maintains these manual lists, as existing code to deal with touching_blablabla won't work here.

edit: HOLY SHIT apparently my VS2015 broke the file formatting (well, it calls it "fixing") when I did ctrl+v. Gotta fix.
https://github.com/coelckers/gzdoom/pull/130

[Graf Zahl]

Looking at the code std::forward_list generates, it doesn't really thrill me. Typical STL: Insanely bloated, borderline unreadable source.
The Boom code has one nice property: It doesn't tear down the entire list, but instead tries to recycle as much of it as possible, and storing deallocated nodes in a free list. std::forward_list is happily allocating stuff from the heap and deleting it later, there's absolutely no way this can be efficient if done more frequently. I already used the msecnode stuff for two other lists without excess code duplication.

It's not urgent here but I'll definitely look into it later to use here, too.

[Graf Zahl]

Oh, and one more thing: This is yet another nail in the coffin of the player prediction code for multiplayer. I have no idea what to do with that. My impression is that it should be done differently anyway but I don't even fully understand what it wants to do, thanks to insufficient documentation.

[ZZYZX]

Some bugs showed up. Sorry for not testing this thoroughly enough before it was merged, currently actively working on it.

• In software, "currently rendered" sector is used as the source of thing's brightness as opposed to thing->Sector. This didn't matter before, but now it means things receive wrong illumination when they are rendered from different sector.
  Currently I see only one way of fixing this — duplicating some of the R_FakeFlat logic per-thing to calculate their own light levels (but only if the thing is linked to more than one sector).
• OpenGL crashes. OpenGL crashes in an extremely weird way, only after some time playing, claiming that on line 373 thing is nullptr. Which is plainly not possible because if thing was nullptr it wouldn't get added to the list in the first place. edit: In debug build, it always crashes like this.
  Tested by playing TUTNT for 15 minutes in software, no crash. Testing in OpenGL, always (in debug build) receiving the same crash after killing second hellknight on spawn of TUTNT TNT01.
  Graf's opinion on this would be appreciated, as I don't even know where to start. It looks like some actor is M_Free'd without calling Destroy/UnlinkFromWorld (along with P_UnlinkRenderSectors) somehow. BUT ONLY IN OPENGL.
  UPDATE: by logging all linked/unlinked actors, I've narrowed the issue down to actors deriving from class DynamicLight, for example:

  Code: Select all

  actor 1239B4A8 linked in (as DynamicLight) - 1 sectors
  actor 1239B4A8 linked out - 1 sectors

  Yet afterwards it crashes anyway while trying to render actor 1239B4A8. Definitely awaiting Graf's input. He knows his dynamic lights better.
• Major Cooke also reported some extreme slowdowns, but hasn't yet told me the name of the wad to test against. Waiting on that.
  UPDATE: The slowdowns don't reproduce for me using a combination of AEoD+Chillax. Waiting until crashes are fixed, to test again.

[Major Cooke]

Using AEoD, it causes slowdowns. Testing on D4D now.

[ReX]

... if you don't want your physical collision box to be large.

The problem with using a radius as the measure for blocking is that, while it works well for circular (even square) actors, it works pretty badly for elongated actors. Enjay's example of a pipe illustrates this problem very well. Thankfully, we can use invisible 3D sectors or invisible bridge things for proper blocking.

But, it sounds like the RenderRadius method will solve the underlying problem of odd-shaped or long models disappearing. Many thanks for tackling this visual glitch.

[Graf Zahl]

Some bugs showed up. Sorry for not testing this thoroughly enough before it was merged, currently actively working on it.
In software, "currently rendered" sector is used as the source of thing's brightness as opposed to thing->Sector. This didn't matter before, but now it means things receive wrong illumination when they are rendered from different sector.
Currently I see only one way of fixing this — duplicating some of the R_FakeFlat logic per-thing to calculate their own light levels (but only if the thing is linked to more than one sector).

That's bad. It took me quite a while to find out where it messes up and it's just business as usual for the software renderer: Fetching data from some global variable that gets set in an entirely unrelated part of the code. The global variable in question here is 'basecolormap'. This is stuff I once gave up upon when trying to add per-tier coloring of walls, because it's utterly messed up.
I marked the place in the code where this needs to be handled, but I'm sorry, this exceeds my comfort level with the software renderer.

[ZZYZX]

Anything on the dynlights though?

[Graf Zahl]

ADynamicLight requires setting RenderRadius to -1 anyway so that should solve this problem. I also wonder if that may have been the cause of the slowdowns because lights can get quite large.
Aside from that I'm still waiting for feedback if the latest changes have improved matters.

[Rachael]

What mod do you want to test with? I will compile a build myself.

[Graf Zahl]

No idea. I need some info about stuff that caused a slowdown which I can reproduce. None of the people who reported it said whether they tested with software and hardware rendering and the dynamic light issue definitely was causing problems.

[Major Cooke]

I'm working on it now.

[ZZYZX]

Latest GZDoom code doesn't crash for me anymore.

ADynamicLight requires setting RenderRadius to -1 anyway so that should solve this problem.

Disabling the additional sectors was not solving the problem for me before. Also, this would still mean that if someone somehow sets RenderRadius for dynamic lights it will start to crash again.
I mean, it'd still be nice to understand why DynamicLight was unlinked successfully from it's sector, but then still attempted to render. And why this doesnt happen to any other kind of actors.
Anyway seeing as it got fixed when you deleted the forward_list, it was probably some unobvious bug with the allocators or something.

Also, unless you find yourself interested in it first, I might try to fix the software lightlevel error myself. Later. Tomorrow or so.

[Major Cooke]

It appears the performance drops have been fixed. Still doing more testing but it feels definitely better now.