Skip to content

Commit

Permalink
Fewer submap inbounds checks by using _ib coords
Browse files Browse the repository at this point in the history
Improve performance by removing inbounds-checks when we have already
checked before that a point is inbound.

This main intention of this commit is to remove the inbounds check from
`map::get_nonant` by forcing the caller to supply an
already-inbounds-checked tripoint using an `_ib` type. But in order to
do that, two other changes are required (in this commit):

1. Change submap coordinates from `tripoint_rel_sm`->`tripoint_bub_sm`.
2. Update calls to `get_submap_at_grid` to specify whether the point
   is already inbounds-checked or not.

These two changes are described below:

1. Change to `tripoint_bub_sm`

So, we want to use one of the `_ib` types.

Coordinates to specify a position for a submap used `tripoint_rel_sm`
before this commit. But `_rel_sm` has no inbounds `_ib` type. It
shouldn't. Saying that a relative coordinate is inbounds doesn't make
sense - it only makes sense if we also specify another point to where it
is relative to. One such point is the bubble reference.

This commit therefore changes coordinates that specify a submap so that
they instead use `tripoint_bub_sm`. This is in fact a stronger statement
about the coordinate type - now we're saying that the coordinate is
relative to the bubble. Before, when using `tripoint_rel_sm`, the type
did not explicitly state what it was relative to (but before, it was
always implicitly relative to the bubble anyway).

`_bub_sm` also has an `_ib` type, so this commit uses
`tripoint_bub_sm_ib` to specify coordinates for submaps where we've
already inbounds-checked it before.

2. Update calls to `get_submap_at_grid`

The method `get_submap_at_grid` takes a submap coordinate and returns
the submap. In some of the places where we call this method, we can be
sure that the point is in fact inbounds, for example:

* when having called `inbounds()` just before, such as in
  `map::get_submap_at( const tripoint & )`
* When iterating xy between `0` and `my_MAPSIZE`

This commit therefore updates the places where it is obvious that the
point is already bounds-checked so that `get_submap_at_grid` is called
with `tripoint_bub_sm_ib` (inbound).

The other overloaded version of `get_submap_at_grid` that takes
`tripoint_bub_sm` still does inbounds checking.
  • Loading branch information
inogenous committed Aug 19, 2024
1 parent 4314562 commit 3ff4b33
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 130 deletions.
17 changes: 10 additions & 7 deletions src/editmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,8 @@ void editmap::draw_main_ui_overlay()
std::map<tripoint_bub_ms, std::tuple<mtype_id, int, bool, Creature::Attitude>> spawns;
for( int x = 0; x < 2; x++ ) {
for( int y = 0; y < 2; y++ ) {
submap *sm = tmpmap.get_submap_at_grid( { x, y, target.z()} );
const tripoint_bub_sm_ib submap_pos{ x, y, target.z() };
submap *sm = tmpmap.get_submap_at_grid( submap_pos );
if( sm ) {
const tripoint_bub_ms sm_origin = origin_p + tripoint( x * SEEX, y * SEEY, target.z() );
for( const spawn_point &sp : sm->spawns ) {
Expand Down Expand Up @@ -1879,7 +1880,7 @@ void editmap::mapgen_preview( const real_coords &tc, uilist &gmenu )
} else if( gpmenu.ret == 1 ) {
tmpmap.rotate( 1 );
} else if( gpmenu.ret == 2 ) {
const point_rel_sm target_sub( target.x() / SEEX, target.y() / SEEY );
const point_bub_sm target_sub( target.x() / SEEX, target.y() / SEEY );

here.set_transparency_cache_dirty( target.z() );
here.set_outside_cache_dirty( target.z() );
Expand All @@ -1894,8 +1895,8 @@ void editmap::mapgen_preview( const real_coords &tc, uilist &gmenu )
for( int z = -OVERMAP_DEPTH; z <= OVERMAP_HEIGHT; z++ ) {
// Apply previewed mapgen to map. Since this is a function for testing, we try avoid triggering
// functions that would alter the results
const tripoint_rel_sm dest_pos = target_sub + tripoint( x, y, z );
const tripoint_rel_sm src_pos = tripoint_rel_sm{ x, y, z };
const tripoint_bub_sm dest_pos = target_sub + tripoint( x, y, z );
const tripoint_bub_sm_ib src_pos = tripoint_bub_sm_ib{ x, y, z };

submap *destsm = here.get_submap_at_grid( dest_pos );
submap *srcsm = tmpmap.get_submap_at_grid( src_pos );
Expand All @@ -1921,7 +1922,7 @@ void editmap::mapgen_preview( const real_coords &tc, uilist &gmenu )
// Since we cleared the vehicle cache of the whole z-level (not just the generate map), we add it back here
for( int x = 0; x < here.getmapsize(); x++ ) {
for( int y = 0; y < here.getmapsize(); y++ ) {
const tripoint_rel_sm dest_pos = tripoint_rel_sm( x, y, target.z() );
const tripoint_bub_sm_ib dest_pos = tripoint_bub_sm_ib( x, y, target.z() );
const submap *destsm = here.get_submap_at_grid( dest_pos );
if( destsm == nullptr ) {
debugmsg( "Tried to update vehicle cache at (%d,%d,%d) but the submap is not loaded", dest_pos.x(),
Expand Down Expand Up @@ -1968,7 +1969,8 @@ vehicle *editmap::mapgen_veh_query( const tripoint_abs_omt &omt_tgt )
std::vector<vehicle *> possible_vehicles;
for( int x = 0; x < 2; x++ ) {
for( int y = 0; y < 2; y++ ) {
submap *destsm = target_bay.get_submap_at_grid( { x, y, target.z()} );
const tripoint_bub_sm_ib submap_pos{ x, y, target.z() };
submap *destsm = target_bay.get_submap_at_grid( submap_pos );
if( destsm == nullptr ) {
debugmsg( "Tried to get vehicles at (%d,%d,%d) but the submap is not loaded", x, y, target.z() );
continue;
Expand Down Expand Up @@ -2005,7 +2007,8 @@ bool editmap::mapgen_veh_destroy( const tripoint_abs_omt &omt_tgt, vehicle *car_
target_bay.load( omt_tgt, false );
for( int x = 0; x < 2; x++ ) {
for( int y = 0; y < 2; y++ ) {
submap *destsm = target_bay.get_submap_at_grid( { x, y, target.z()} );
const tripoint_bub_sm_ib submap_pos{ x, y, target.z() };
submap *destsm = target_bay.get_submap_at_grid( submap_pos );
if( destsm == nullptr ) {
debugmsg( "Tried to destroy vehicle at (%d,%d,%d) but the submap is not loaded", x, y, target.z() );
continue;
Expand Down
6 changes: 4 additions & 2 deletions src/lightmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ bool map::build_transparency_cache( const int zlev )
// Traverse the submaps in order
for( int smx = 0; smx < my_MAPSIZE; ++smx ) {
for( int smy = 0; smy < my_MAPSIZE; ++smy ) {
const submap *cur_submap = get_submap_at_grid( {smx, smy, zlev} );
const tripoint_bub_sm_ib submap_pos{ smx, smy, zlev };
const submap *cur_submap = get_submap_at_grid( submap_pos );
if( cur_submap == nullptr ) {
debugmsg( "Tried to build transparency cache at (%d,%d,%d) but the submap is not loaded", smx, smy,
zlev );
Expand Down Expand Up @@ -435,7 +436,8 @@ void map::generate_lightmap( const int zlev )
// Traverse the submaps in order
for( int smx = 0; smx < my_MAPSIZE; ++smx ) {
for( int smy = 0; smy < my_MAPSIZE; ++smy ) {
const submap *cur_submap = get_submap_at_grid( { smx, smy, zlev } );
const tripoint_bub_sm_ib submap_pos{ smx, smy, zlev };
const submap *cur_submap = get_submap_at_grid( submap_pos );
if( cur_submap == nullptr ) {
debugmsg( "Tried to generate lightmap at (%d,%d,%d) but the submap is not loaded", smx, smy, zlev );
continue;
Expand Down
Loading

0 comments on commit 3ff4b33

Please sign in to comment.