Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial work on 3D vision #13595

Merged
merged 33 commits into from
Jan 12, 2016
Merged

Conversation

Coolthulhu
Copy link
Contributor

Mostly pushing so that I can get a design issue/question resolved:
Current line_to does diagonal moves. This is OK for x/y, but diagonal moves that include z-level moves make my job harder.
Is it OK to have line_to split horizontal (x/y) and vertical moves into one-then-other? Is my version (the one in the diff) of splitting it correct?

Now the actual summary of the PR:

  • Makes map::build_map_cache build transparency and outside caches for all z-levels (in z-level mode). This is actually a minor bugfix, as it could have been enabling some subtle (minor) bugs.
  • Makes the game not recalculate sight cache when looking around (in look mode) when it doesn't change (still recalculates a lot, there's room for improvement)

Adds an option in debug tab. Setting it on enables the stuff below:

  • 3D map::sees and map::find_clear_path. Those work like 2D ones except they use 3D bresenham, and when making a z-move they check for m.valid_move to determine if it can actually be made.
  • Adds a slow raycasting 3D FoV calculation that is used only when the debug option is on. It checks all potentially visible tiles. Based on a stripped-down version of the proper shadowcasting algorithm. Megatons of room for improvement here, this is just for testing for now.

For now I'll be going for #6824 (sight and attacks) rather than #6821 (actual FoV itself), as the former is both more noticeable and easier to implement.

@kevingranade
Copy link
Member

What about diagonal+vertical offsets makes things harder?
line_to() should just draw a bresenham line to the target, if something needs some other constraints, it needs to be a separate function. It was a real mess cleaning out the weirdness around that from the original.

return !valid_move( p, below, false, true );
};

// Raycasting, as opposed to shadowcasting for 2D
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raycasting is immensely slower than shadowcasting (and it has weird artifacts), I'm extremely skeptical that this will be a workable solution for anything other than working on the draw code. The 3d FoV code is going to need to be either shadowcasting, or something more sophisticated than shadowcasting. Probably we can get by with shadowcasting plus some z-level aware fixups.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's why I am going for the part 5 with it first - just for sight and attacks, not as the FoV function.

Updating shadowcasting to 3D would require some clever algorithm - either extending shadowcasting to 3D (which would probably be a totally new algorithm) or tolerating some simplifications and asymmetries during z moves.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, it was late and I missed that part of your summary.
I think extending shadowcasting to 3D is doable, I have a mental model for it in my head.
The shadowcasting algorithm works by finding spans of adjacent squares within an octant and processing them recursively. It does this by iteratively considering each square and deciding whether to add it to the current span, or use it as the start of a new span. As an optimization (and the source of the name) spans with no visibility are not processed. Before the algorithm processes a new span, it recursively processes the previous span.
To extend the algorithm to 3D, the algorithm would need to find 3D spans (frustums) with uniform transparency, it can do this by iterating along a major and minor axis, when a change in transparency is found, the algorithm will need to split the existing frustum along the major axis (creating a frustum with depth along both the major and minor axis) and split the previous segment of the current iteration along the minor axis (creating a frustum with no width along the major axis, and 0 or more width along the minor axis), the algorithm would recurse both frustums, then proceed to the new span. I can see it in my head, but it's really hard to explain :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean.
I had a similar idea.

Well, I'm not sure if I get what minor and major axes are here, but otherwise I think I get it.
If minor and major axes are arbitrarily chosen (like now y vs x in castLight) rather than depending on some factor, then I'm pretty sure I get it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One final check if I really get it:
We want to split the currently processed projection/rectangle/etc. into 3 parts:

  • Rectangle that is already processed
  • Rectangle that has one line processed, rest unprocessed. The line ends with (before? after?) a point with change in transparency
  • Rectangle that is unprocessed

Then extend a frustum (another recursive call) from each of those rectangles.

Right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chaosvolt
Copy link
Contributor

Oh wow, this gets more impressive by the day.

@Coolthulhu
Copy link
Contributor Author

What about diagonal+vertical offsets makes things harder?

A horizontal move has a known vision check (check transparency of the destination tile), vertical move also has a known vision check (existence of the floor on the upper tile).
Horizontal+vertical needs to be split into horizontal+vertical, with the original order of those not being known (ie. in bresenham it could be horizontal+vertical, vertical+horizontal or even h+v+h, but it can't be easily discerned on the outside).

@Coolthulhu
Copy link
Contributor Author

line_to() should just draw a bresenham line to the target, if something needs some other constraints, it needs to be a separate function.

I'll split the line_to and bresenham into their original and modified versions then.
This presents a bit of a maintenance problem, though: functions like monster attacks, vision, projectiles, any sort of movement etc. will have to use the modified versions, the regular "diagonal z" bresenham will not actually be used in many functions.

@Coolthulhu
Copy link
Contributor Author

OK, split the bresenham and line_to into the old one and the modified one.

Added an option to see critters below the player, but it requires either the fov_3d or debug_mode to be on. I can't easily add a test for clairvoyance or DEBUG_VISION here, as those take quite a bit of time and aren't cached (actually, clair is cached, just that this cache is not accessible in Creature::sees or player::sees).

Added drawing a symbol for "critter is below here". A green 'v' on cyan background. Only in fov_3d and debug modes for now.

@Coolthulhu Coolthulhu changed the title [WiP][CR]Initial work on 3D vision Initial work on 3D vision Sep 19, 2015
@kevingranade
Copy link
Member

I'm still not clear on the need for a modified bresenham, I'll read the code and try to figure it out, but I figured I'd try and clarify in prose first.
one, both diagonal+vertical and on-the-same-level+vertical (i.e. x+y+z vs x+z or y+z) have been mentioned, are these separate issues or the same issue?
Setting aside diagonal vs orthogonal, is the problem with transitioning horizontally and vertically at the same time that the order-of-operations for the checks is poorly-defined? I can see there being a problem there, if moving up, you need to check for a roof above the source tile, but if moving down you need to check for a roof above the destination tile. That seems doable in the caller if they have a normally rendered bresenham line, but I might be missing something.

The underlying problem is that horizontally we're using a cell or voxel model, but we're not using that model vertically, things that aren't cells can block vertical movement. Therefore it's not sufficient to check the identity of the target square.

@Coolthulhu
Copy link
Contributor Author

one, both diagonal+vertical and on-the-same-level+vertical (i.e. x+y+z vs x+z or y+z) have been mentioned, are these separate issues or the same issue?

Just horizontal+vertical, whether the horizontal part itself is diagonal or not doesn't really change anything.

is the problem with transitioning horizontally and vertically at the same time that the order-of-operations for the checks is poorly-defined

Mostly this.

I can see there being a problem there, if moving up, you need to check for a roof above the source tile, but if moving down you need to check for a roof above the destination tile. That seems doable in the caller if they have a normally rendered bresenham line, but I might be missing something.

Moving (walking) is a relatively easy case, as it can be assumed that the higher point has a floor.
Consider vision, a flying bullet or even a flying creature. Those may want to execute the diagonal move by going horizontally first.

Additionally, the horizontal move still has to be checked. This leads to an additional split that looks like this:

if( z_level_changed ) {
    tripoint midpoint( new_point.x, new_point.y, old_point.z );
    return valid_move( old_point, midpoint ) && valid_move( midpoint, new_point );
}

Which would further slow down processing of "diagonally vertical" lines.

@Rivet-the-Zombie
Copy link
Member

Awesome stuff, @Coolthulhu!

@kevingranade
Copy link
Member

I think I see, boiling it down to having floating floors above the player's head (between z-levels 0 and 1), how do you decide which squares you can interact with via vision or a thrown object. If there is a ceiling immediately above you, does that mean you can't interact with the 8 squares horizontally adjacent to you but one square up? Similarly, if there is a ceiling above each adjacent square, but not one directly above you, can you still interact with those squares? Intuitively the second must be true, since it corresponds to a player standing in a pit and trying to interact with things adjacent to the pit. the question is, do we also want the first to be true, I think so.
I still think we don't want that difference encoded in the line-drawing function, we want it in things that call it, like creature::sees(creature) and similar. This roughly corresponds to the thing where sees() used to call line_to() with a bunch of different start points to scan for a viable path.

Which would further slow down processing of "diagonally vertical" lines.

I don't think there's a good way around that, your alternative extends the length of the path, which means it processes more squares instead of slightly increasing the cost per-square.

I think if deciding if 0,0,0 (source) can see/attack 1,0,1 (destination), you need to do something like:

if( source.z != destination.z ) {
    // Whether we're going up or down, the floor between source and destination is at the higher of the two z levels.
    int floor_z = std::max(source.z, destination.z);
    // Check for a clear path headed from the source path through the square at source x,y and with the z of the destination (z-first) or through the square at destination x,y with the z of the source (z/y-first).
    if( ( !floor_at(destination.x, destination.y, floor_x) &&
          trans(destination.x, destination.y, source.z) ) ||
        ( !floor_at(source.x, source.y, floor_z) &&
          trans(source.x, source.y, destination.z) ) ) {
        path_clear = true;
    }
}

@Coolthulhu Coolthulhu changed the title Initial work on 3D vision [CR]Initial work on 3D vision Sep 20, 2015
@Coolthulhu
Copy link
Contributor Author

Now I've got a design problem:
The vision algorithm needs a 3D array.
Transparency cache is a 2D array, there is no 3D transparency cache.

I'm passing an array of pointers to 2D arrays, but that's kinda ugly.

Any better way? Preferably (but not necessarily) not involving redesigning the entire cache or dropping the ability to use the same function to project vision and lights.

@kevingranade
Copy link
Member

If you redeclare the cache as a float transparency_cache[NUM_ZLEVELS][MAPSIZE*SEEX][MAPSIZE*SEEY]; I think you you can pass a particular zlevel of it with &transparency_cache[z][0][0] for the 2D code, and pass the whole thing for the 3D code, sound good? That means either updating all the 2D methods to extract that field, or making an alias for it for either 2D or 3D.

You might have to cast the resulting pointer to pass it as a sized array instead of as a raw pointer, haven't checked.

@Coolthulhu Coolthulhu changed the title [CR]Initial work on 3D vision [WiP][CR]Initial work on 3D vision Sep 24, 2015
@Coolthulhu
Copy link
Contributor Author

The implementation seems to be going well, but I still got some bugs to fix.
I have some problems with how to properly do the recursion/split the processed area.

I iterate over y, then z, then x. This means that the area is processed in squares/rectangles up to d tiles tall and up to d tiles wide, where d is the square distance to player.
y and x aren't necessarily y and x in game, because the shadowcasting algorithm can be rotated. z coord is always z coord (for now?).

A given rectangle being processed looks like this:

CCCDDD
CCCDDD
BBBDDD
AAAAAA
AAAAAA
  • A - the rectangle already processed. Here we can safely recurse with y distance increased by 1
  • B - a line (thickness 1 rectangle) already processed that can't be a part of A because it isn't as wide.
  • C - the rectangle not yet processed above B. Could possibly contain B?
  • D - the rectangle not yet processed not above B.

When I encounter a transparency change, I recurse on A, B and C, then process D in current function.
A and B have distance (y coord displacement) increased by 1 compared to currently processed rectangle. C and D are clipped.

Anything I'm doing wrong? Should I document the algorithm somewhere?

@kevingranade
Copy link
Member

kevingranade commented Sep 24, 2015 via email

@Coolthulhu
Copy link
Contributor Author

In case anyone is wondering why is it going so slowly, I'm having problems with what looks like the last edge case:

The FoV edge case

What I figured out:

  • It only happens when looking up/down at least 2 z-levels (image is how it looks when looking from a 2 z-level tall building)
  • It doesn't happen to some near edges, only to ones further away. Those edges which are drawn properly share the common trait: one of their ends can be reached by going only in diagonals from player's tile. Suggests recursion being wrong?
  • Didn't manage to fix it by fiddling with slope precision

This is the worst kind of programming - "shotgun programming". I'm at the point where I'm changing (semi)random stuff and checking that it still doesn't work.

@chaosvolt
Copy link
Contributor

Oh wow. All of this stumping me is expected, but this takes the cake.

@kevingranade
Copy link
Member

I caused a bug resembling this many times when fiddling with the shadowcasting code, it was always due to me accidentally resetting newStart.


const auto mirror_pos = veh->global_pos() + veh->parts[mirror].precalc[0];
// Down
cast_zlight<0, 1, 0, 1, 0, 0, -1, sight_calc, sight_check>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All calls to cast_zlight have the same value for the template parameter xz and yz: 0. Is that intended? Is there actually any need for those parameters (extensions planed for the future)? Also, the // Up and // Down comments are strange, why are there 10 calls to cast_zlight after the // Down, but only 6 calls to it below the // Up comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All calls to cast_zlight have the same value for the template parameter xz and yz: 0. Is that intended? Is there actually any need for those parameters (extensions planed for the future)?

I added them before I knew what I needed, so they may actually be useless. I thought I might be able to use them to map the small cones directly above and directly below the player, but it doesn't really sound like it would work.

Also, the // Up and // Down comments are strange

I got the comments wrong there, what matters is the zz template parameter. // Up should be above the first cast_zlight with zz == 1.

@Coolthulhu
Copy link
Contributor Author

While this isn't done, it performs tolerably for most cases.
I think it can be tested and merged. I'll come back to finish it later, after #6824, which is easier than fixing all the edge cases in FoV (in fact, I already did most of #6824 while implementing other stuff).

Current issues are:

  • "Jaggies" when looking up or down near a wall. Something with new slopes not being consistent
  • Slow
  • When looking up, all terrain with floors is obscured. You couldn't see a hulk looking at you from the roof.

@stk2008
Copy link

stk2008 commented Jan 9, 2016

Ohhhh cant wait to test this one :)

On Sat, Jan 9, 2016 at 11:08 AM, Coolthulhu [email protected]
wrote:

While this isn't done, it performs tolerably for most cases.
I think it can be tested and merged. I'll come back to finish it later,
after #6824 #6824,
which is easier than fixing all the edge cases in FoV (in fact, I already
did most of #6824
#6824 while
implementing other stuff).

Current issues are:

  • "Jaggies" when looking up or down near a wall. Something with new
    slopes not being consistent
  • Slow
  • When looking up, all terrain with floors is obscured. You couldn't
    see a hulk looking at you from the roof.


Reply to this email directly or view it on GitHub
#13595 (comment)
.

@kevingranade
Copy link
Member

Segfault, go to an office tower in a release build, turn on debug mode, and try to look at z level 2.
Can't get anything out of the coredump, rebuilding with debug symbols.

@Rivet-the-Zombie
Copy link
Member

Always looking forward to more Z-level implementation.

Very nice, Coolthulhu!

@kevingranade
Copy link
Member

Segfault isn't a problem with the new code, drawing a level the player isn't on breaks an assumption of some new code. (cata_tiles.cpp:1314)
It's trying to examine some data from a vehicle that's not on the current level.

@kevingranade
Copy link
Member

I'm hacking away at this, it ended up rather involved because there are a number of method calls that need a tripoint instead of a pair of x/y coordinates.

@chaosvolt
Copy link
Contributor

I'm not sure if that statement is implying a cause or a hindrance towards finding said cause.

@kevingranade kevingranade merged commit ccf72ac into CleverRaven:master Jan 12, 2016
@chaosvolt
Copy link
Contributor

Ah, nice to see that in. If I recall, this was the main thing keeping reality-devouring darkness ( #13786 ) in the "long term" pile.

@stk2008
Copy link

stk2008 commented Jan 12, 2016

Ah was this PR to allow us to say..stand ontop of a tall building and see
the road etc below?

If so it dont work...well in TILES version :(

On Tue, Jan 12, 2016 at 8:37 AM, Chaosvolt [email protected] wrote:

Ah, nice to see that in. If I recall, this was the main thing keeping
reality-devouring darkness ( #13786
#13786 ) in the
"long term" pile.


Reply to this email directly or view it on GitHub
#13595 (comment)
.

@TheRafters
Copy link
Contributor

It lets you use 'x' to look around. you use "<" and ">" to look up and down. The walking around vision is coming, if I'm reading the op correctly.
Oh, and you have to enable the option in the debug menu as well.

@stk2008
Copy link

stk2008 commented Jan 12, 2016

oh damn I got to test this out then.

thanks

On Tue, Jan 12, 2016 at 3:38 PM, Malkeus [email protected] wrote:

It lets you use 'x' to look around. you use "<" and ">" to look up and
down. The walking around vision is coming, if I'm reading the op correctly.


Reply to this email directly or view it on GitHub
#13595 (comment)
.

@stk2008
Copy link

stk2008 commented Jan 12, 2016

Hmmm i press x the <> but it dont work?

On Tue, Jan 12, 2016 at 3:39 PM, daniel roberts [email protected]
wrote:

oh damn I got to test this out then.

thanks

On Tue, Jan 12, 2016 at 3:38 PM, Malkeus [email protected] wrote:

It lets you use 'x' to look around. you use "<" and ">" to look up and
down. The walking around vision is coming, if I'm reading the op correctly.


Reply to this email directly or view it on GitHub
#13595 (comment)
.

@TheRafters
Copy link
Contributor

did you enable 3d vision in the debug menu?

@Coolthulhu
Copy link
Contributor Author

It is disabled by default, even on z-level worlds. You need to explicitly enable it in options.
That's because a lot of people are already playing with z-levels mode on and 3D vision is unfinished, very slow and might get buggy in some places.

@TheRafters
Copy link
Contributor

I'm excited about this one. Does enabling this mean zombies will be able to see me and path to me across z-levels now?

@stk2008
Copy link

stk2008 commented Jan 12, 2016

i got it to work i think

You need to enable it in optoins and then enable DEBUG mode in game then I
can look around Z levels :)

On Tue, Jan 12, 2016 at 3:50 PM, Malkeus [email protected] wrote:

I'm excited about this one. Does enabling this mean zombies will be able
to see me and path to me across z-levels now?


Reply to this email directly or view it on GitHub
#13595 (comment)
.

@Coolthulhu Coolthulhu deleted the slow-3d-fov branch January 12, 2016 15:54
@Coolthulhu
Copy link
Contributor Author

See yes, but pathing may often be quite wonky.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants