[WIP] Fix 3d shadowcasting #32012

ifreund · 2019-06-30T17:50:52Z

Summary

SUMMARY: Bugfixes "Fix bugs and increase performance of 3D shadowcasting"

Purpose of change

I'm attempting to revive #30861 as Kevin is busy with other performance work. Much of this is directly copied from that PR.

Fixes #6821
BOUNTY

3D shadowcasting has a number of rendering bugs remaining, fix them.
A non-exhaustive list:

~~As in Attacks from under ceiling with 3D FOV on. #29175 shockers are sometimes able to see and target you through a floor. With some debugging Kevin determined the cause to be 3D shadowcasting.~~ Was caused by map shifts apparently.
When standing cardinally adjacent to a single opaque tile (wall or adult tree, etc), in 2D shadowcasting the area behind the tree is obscured, but in 3D it is visible.
UPDATE: seems to be fixed with the slope struct.
When standing adjacent to a wall corner, in 2D shadowcasting the first tile of the wall is visible and the rest is occluded, in 3D the entire wall and anything behind it is visible.
UPDATE: seems to be fixed with the slope struct.
3D shadowcasting does not cast directly up or down, so there's a shadow directly above and below the player.
UPDATE: Now fixed. We cast on 8 new octants, 4 up and 4 down.

Describe the solution

This is going to be a progressive process since I don't know what the problem is.

Extend shadowcasting test to trigger 3D shadowcasting and test 3D scenarios.
Add tests that characterize the breakage.
Replace recursive shadowcasting with iterative shadowcasting as outlined in Further shadowcasting optimizations #23373. In addition to being more performant, this should be easier to analyze and test.
Fix an infinite loop that pops up in some cases.
Fix the remaining bugs.
Cast straight up and down

ifreund · 2019-07-04T21:19:09Z

Quick update, I've corrected most of the major bugs and the new iterative 3d FOV is now totally playable and quite a bit more correct that the current recursive algorithm.

The only major todo item I'm currently aware of is doing something about casting straight up/down as we currently don't do that at all (That's the only test failing on travis currently).

ifreund · 2019-07-05T11:28:41Z

src/shadowcasting.cpp

+         T( *calc )( const T &, const T &, const int & ),
+         bool( *check )( const T &, const T & ),
+         T( *accumulate )( const T &, const T &, const int & )>
+void cast_vertical_zlight_segment(


Splitting cast_zlight_segment into vertical and horizontal versions isn't ideal and duplicates a lot of complex code. However, the alternative is some quite nasty iteration logic to handle the full range of possible coordinate transformations instead of the subset currently handled. Splitting it like this makes this already complex code a good bit more readable without the extra conditionals.

Would definitely appreciate feedback on whether this choice makes sense to others though.

It's certainly possible to merge the horizontal and vertical versions of the algorithm. You'd want to abstract out the coordinate system more into some type template argument that would replace the various _transform numbers. Each version of that type would have methods to do the various transformations needed (between depth/major/minor coordinates and x/y/z coordinates).

I think it is worth doing and would probably help clarity and maintainability, but it doesn't have to be on this first PR.

ifreund · 2019-07-05T11:35:00Z

3d shadowcasting now casts directly up and down, with a total of 8 more casts than the previous method. This means no more pyramid of unseen tiles directly above and below the player.

Performance impact shouldn't be too bad, as downward casts will almost always end immediately as they encounter the opaque ground and upward casts will either end immediately if under a roof or end much sooner than horizontal casts due to our rather low maximum z level.

So, upwards casting will be a slight slowdown when outdoors, but allows the player to see all the tiles they are supposed to see. The optimization described in #23914 or something like it would make upward casting trivial in most cases as the area above the player is usually either blocked by a roof or completely open.

kevingranade · 2019-07-05T22:55:53Z

src/shadowcasting.cpp

+        }
+    }
+
+    int rise;


Fun fact, the shadowcasting area is only 60 deep and 60 wide, so rise and run can be 8-bit.
probably int_least8_t or uint_least8_t

Makes sense, I had meant to go back and revise that but forgot.

We do have negative slopes in some valid cases, so I'll be using int_least8_t

FWIW, I think int_least8_t is probably overkill. Cataclysm is already not going to work on any system where that isn't the same as int8_t.

ifreund · 2019-07-06T06:40:32Z

Need to figure out if int_fast8_t would be better for this use case, but that is non-trivial to profile properly and is an small change if needed.

Travis is complaining about a memory leak and I'm not quite sure how that could have happened as a result of these changes, but I'll do some more digging.

tests/shadowcasting_test.cpp

ifreund · 2019-07-06T17:04:50Z

Clany tidy error is unrelated, I'd say this is good to go.

ZhilkinSerg · 2019-07-07T08:18:06Z

Jenkins rebuild.

esotericist · 2019-07-08T02:47:19Z

So, I merged this locally to do some playtests, and there's some kinda weird behavior here with view distance in reduced vision situations. I first noticed it while in a vehicle, but I was able to reproduce it in a building.

Details: it's 5:16 AM, with "drizzle" weather. So it's "very dark" outside.

Default vision (3d fov off) while in vehicle:

After turning on 3d fov, and moving (to reset visible tiles):

Here's the difference between not-3d-fov and yes-3d-fov in the building (also involving steps in between to reset visible tiles):

For extra bonus weird, I noticed view distance changing as I moved within the building. These next two shots are still with 3d fov on, as I step towards the door:

The behavior reverses as I move back to my original position.

Extra extra extra bonus weird: what happens if I move one tile to the right, to sit in the chair:

That last one at least matches the default vision algorithm

jbytheway · 2019-07-08T06:52:52Z

I had to turn off one of the vision tests on 3D because of the inconsistencies. Please turn that back on (indeed, you can totally rip out the feature of being able to turn off particular tests in 3D mode).

jbytheway

Looking good in terms of code quality. I haven't tested it, and the above examples indicate some changes from the previous algorithm, which should be investigated.

I'd like to see the vision tests generalized to 3D as well as the shadowcasting tests. The shadowcasting tests only test the boolean "is it visible at all", whereas the vision tests get into questions like "how visible is it?". I'd like us to be able to verify that there aren't any artefacts at the boundaries between the horizontal and vertical casting algorithms (e.g. look three levels up, is there an obvious 6x6 square where stuff looks different). Not vital that those be added in this PR, though.

src/shadowcasting.cpp

jbytheway · 2019-07-08T07:14:04Z

src/shadowcasting.cpp

+         T( *calc )( const T &, const T &, const int & ),
+         bool( *check )( const T &, const T & ),
+         T( *accumulate )( const T &, const T &, const int & )>
+void cast_vertical_zlight_segment(


It's certainly possible to merge the horizontal and vertical versions of the algorithm. You'd want to abstract out the coordinate system more into some type template argument that would replace the various _transform numbers. Each version of that type would have methods to do the various transformations needed (between depth/major/minor coordinates and x/y/z coordinates).

I think it is worth doing and would probably help clarity and maintainability, but it doesn't have to be on this first PR.

ifreund · 2019-07-08T13:46:21Z

I believe my most recent commit has fixed that bug @esotericist, at least I can't reproduce it anymore. Thanks for testing!

jbytheway · 2019-07-08T16:49:45Z

I believe my most recent commit has fixed that bug @esotericist, at least I can't reproduce it anymore. Thanks for testing!

Did you ensure we have a test that captures that failure mode before fixing it?

tests/vision_test.cpp

ifreund · 2019-07-08T18:29:15Z

Did you ensure we have a test that captures that failure mode before fixing it?

Unfortunately I haven't been able to setup a test case that consistently passes after the change and fails before it, since the failure case is very sensitive to lighting. I've tried setting the weather before the test and doing the test several z levels underground. I can get it to behave properly 4/5 times or so, but haven't managed to track down the inconsistency yet.

kevingranade · 2019-07-09T07:05:03Z

This looks not-right...

ifreund · 2019-07-09T08:30:27Z

Went to the same location and could only reproduce half of the bug. You sure you had 96abcad included in that build?

Screenshots

and remove the code that allows disabling it

It somehow took me until now to realize that y coordinates are counted from the "top" of the screen not the bottom, so fix the comment diagrams

ifreund · 2019-07-09T21:39:56Z

Marking this WIP for now as there's still a reproducible bug with it that I haven't managed to track down yet. In the process of debugging I found some faulty logic that may be partially at fault and am in the process of reworking how spans get split.

ThomasLinkin · 2019-09-30T08:40:13Z

so this is abandoned i suppose?

esotericist · 2019-09-30T09:17:37Z

so this is abandoned i suppose?

Not abandoned, merely stalled. The people with the appropriate expertise have been dealing with other matters, be it within the project or things outside of the project.

This will get tended to eventually.

ifreund · 2020-09-28T22:22:00Z

Superseded by 42835

Brambor · 2023-11-25T13:58:56Z

Superseded by #42835

Add # so that I can click it :)

ifreund added Code: Performance Performance boosting code (CPU, memory, etc.) Z-levels Levels below and above ground. [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) labels Jun 30, 2019

ifreund force-pushed the fix-3d-shadowcasting-2 branch from 83bf5d0 to 5e031a2 Compare July 4, 2019 18:37

ifreund force-pushed the fix-3d-shadowcasting-2 branch 3 times, most recently from e7d0a5f to f83913b Compare July 5, 2019 11:24

ifreund marked this pull request as ready for review July 5, 2019 11:25

ifreund commented Jul 5, 2019

View reviewed changes

ifreund force-pushed the fix-3d-shadowcasting-2 branch 2 times, most recently from 07765bf to dcd3607 Compare July 5, 2019 14:20

kevingranade reviewed Jul 5, 2019

View reviewed changes

ZhilkinSerg self-assigned this Jul 6, 2019

kevingranade reviewed Jul 6, 2019

View reviewed changes

tests/shadowcasting_test.cpp Outdated Show resolved Hide resolved

ifreund mentioned this pull request Jul 7, 2019

Introduce a faster linked list implementation #32234

Merged

jbytheway reviewed Jul 8, 2019

View reviewed changes

tests/vision_test.cpp Show resolved Hide resolved

ifreund added 4 commits July 9, 2019 16:28

Cleanup template parameters

9e93eff

Test 3d fov in all vision tests

1f2edfb

and remove the code that allows disabling it

Rename check to is_transparent and fix comments

5b20d45

It somehow took me until now to realize that y coordinates are counted from the "top" of the screen not the bottom, so fix the comment diagrams

Fix erroneous cumulative transparency incrementation

0ae920b

ifreund force-pushed the fix-3d-shadowcasting-2 branch from 5289562 to 0ae920b Compare July 9, 2019 14:30

ifreund changed the title ~~Fix 3d shadowcasting~~ [WIP] Fix 3d shadowcasting Jul 9, 2019

ZhilkinSerg removed their assignment Jul 13, 2019

WIP rewrite of span splitting

494a33b

ifreund force-pushed the fix-3d-shadowcasting-2 branch from 4403e33 to 494a33b Compare July 14, 2019 13:11

Fix some of the bugs

9236a76

kevingranade mentioned this pull request Aug 11, 2019

Extreme performance hit heading into city with 3d FOV enabled #31952

Closed

weekly-digest bot mentioned this pull request Dec 29, 2019

Weekly Digest (22 December, 2019 - 29 December, 2019) #36542

Closed

ZhilkinSerg added 0.E Feature Freeze and removed 0.E Feature Freeze labels Jan 30, 2020

weekly-digest bot mentioned this pull request Feb 2, 2020

Weekly Digest (26 January, 2020 - 2 February, 2020) #37641

Closed

kevingranade added the 0.E Feature Freeze label Feb 11, 2020

This was referenced Feb 16, 2020

Weekly Digest (9 February, 2020 - 16 February, 2020) #38055

Closed

Weekly Digest (9 February, 2020 - 16 February, 2020) #38058

Closed

ifreund removed the 0.E Feature Freeze label Apr 4, 2020

ifreund marked this pull request as draft April 15, 2020 16:42

weekly-digest bot mentioned this pull request Apr 19, 2020

Weekly Digest (12 April, 2020 - 19 April, 2020) #39724

Closed

ZhilkinSerg added the 0.F Feature Freeze label Aug 3, 2020

ifreund closed this Sep 28, 2020

kevingranade mentioned this pull request Sep 28, 2020

Obscure 3D FoV bug #44497

Open

ifreund mentioned this pull request Sep 29, 2020

Use plf linked list impl for shadowcasting #44512

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Fix 3d shadowcasting #32012

[WIP] Fix 3d shadowcasting #32012

ifreund commented Jun 30, 2019 •

edited

Loading

ifreund commented Jul 4, 2019

ifreund Jul 5, 2019 •

edited

Loading

jbytheway Jul 8, 2019 •

edited

Loading

ifreund commented Jul 5, 2019

kevingranade Jul 5, 2019

ifreund Jul 6, 2019

ifreund Jul 6, 2019

jbytheway Jul 8, 2019

ifreund commented Jul 6, 2019

ifreund commented Jul 6, 2019

ZhilkinSerg commented Jul 7, 2019

esotericist commented Jul 8, 2019

jbytheway commented Jul 8, 2019

jbytheway left a comment •

edited

Loading

jbytheway Jul 8, 2019 •

edited

Loading

ifreund commented Jul 8, 2019

jbytheway commented Jul 8, 2019

ifreund commented Jul 8, 2019

kevingranade commented Jul 9, 2019

ifreund commented Jul 9, 2019

ifreund commented Jul 9, 2019

ThomasLinkin commented Sep 30, 2019

esotericist commented Sep 30, 2019

ifreund commented Sep 28, 2020

Brambor commented Nov 25, 2023

[WIP] Fix 3d shadowcasting #32012

[WIP] Fix 3d shadowcasting #32012

Conversation

ifreund commented Jun 30, 2019 • edited Loading

Summary

Purpose of change

Describe the solution

ifreund commented Jul 4, 2019

ifreund Jul 5, 2019 • edited Loading

Choose a reason for hiding this comment

jbytheway Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

ifreund commented Jul 5, 2019

kevingranade Jul 5, 2019

Choose a reason for hiding this comment

ifreund Jul 6, 2019

Choose a reason for hiding this comment

ifreund Jul 6, 2019

Choose a reason for hiding this comment

jbytheway Jul 8, 2019

Choose a reason for hiding this comment

ifreund commented Jul 6, 2019

ifreund commented Jul 6, 2019

ZhilkinSerg commented Jul 7, 2019

esotericist commented Jul 8, 2019

jbytheway commented Jul 8, 2019

jbytheway left a comment • edited Loading

Choose a reason for hiding this comment

jbytheway Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

ifreund commented Jul 8, 2019

jbytheway commented Jul 8, 2019

ifreund commented Jul 8, 2019

kevingranade commented Jul 9, 2019

ifreund commented Jul 9, 2019

ifreund commented Jul 9, 2019

ThomasLinkin commented Sep 30, 2019

esotericist commented Sep 30, 2019

ifreund commented Sep 28, 2020

Brambor commented Nov 25, 2023

ifreund commented Jun 30, 2019 •

edited

Loading

ifreund Jul 5, 2019 •

edited

Loading

jbytheway Jul 8, 2019 •

edited

Loading

jbytheway left a comment •

edited

Loading

jbytheway Jul 8, 2019 •

edited

Loading