Lag on scrolling map with git build

[pingpong]

Just like it reads, not game-breaking, but any ideas? My viewport is 70x50.

[Soron]

Known bug that we would like help with: https://github.com/TheDarklingWolf/Cataclysm-DDA/issues/713

It was introduced when we bumped the cap on overmap terrain types (which in turn allowed us to add a bunch of acidia’s awesome new buildings). The fix would either be to reduce the number of save/load operations that are needed, or speed up the way that we’re handling overmap saving and loading.

[LazyCat]

Just like it reads, not game-breaking, but any ideas? My viewport is 70x50.

You mean the map when you press ‘m’? Only near map borders, or everywhere and all the time?

[LazyCat]

[quote=“Soron, post:2, topic:1104”]Known bug that we would like help with: https://github.com/TheDarklingWolf/Cataclysm-DDA/issues/713

It was introduced when we bumped the cap on overmap terrain types (which in turn allowed us to add a bunch of acidia’s awesome new buildings). The fix would either be to reduce the number of save/load operations that are needed, or speed up the way that we’re handling overmap saving and loading.[/quote]

If we are talking about the same lag it should be happening only near overmap borders.

I eliminated lag when viewing map (‘m’) by commenting out this block of code in overmap.cpp, void overmap::draw

/*
  if (cursx < om_map_width / 2) {
   hori = overmap(g, loc.x - 1, loc.y);
   if (cursy < om_map_height / 2)
    diag = overmap(g, loc.x - 1, loc.y - 1);
   if (cursy > OMAPY - 2 - (om_map_height / 2))
    diag = overmap(g, loc.x - 1, loc.y + 1);
  }
  if (cursx > OMAPX - 2 - (om_map_width / 2)) {
   hori = overmap(g, loc.x + 1, loc.y);
   if (cursy < om_map_height / 2)
    diag = overmap(g, loc.x + 1, loc.y - 1);
   if (cursy > OMAPY - 2 - (om_map_height / 2))
    diag = overmap(g, loc.x + 1, loc.y + 1);
  }
  if (cursy < (om_map_height / 2))
   vert = overmap(g, loc.x, loc.y - 1);
  if (cursy > OMAPY - 2 - (om_map_height / 2))
   vert = overmap(g, loc.x, loc.y + 1);
*/

…there are few side-effects, but I actually like them. Anyway, when near overmap border there is also lag when moving the character on filed-view screen. I don’t see why would few more tiles or storing/reading int instead of char make such drastic difference. It seems to me the game is loading/saving the whole map when it really shouldn’t. I mean when you come close to border you need to load neighbouring map just once, not every time character moves to another overmap tile, which is what seems to be happening.

I also find it’s peculiar change new overload operator is introduced:

overmap& overmap::operator=(overmap const& o)

I have no idea how it worked before without it, but I think the problem could have something to do with it.

Can someone explain how was that whole thing working before without overload operator and why was it introduced?

[Soron]

It seems to me the game is loading/saving the whole map when it really shouldn’t. I mean when you come close to border you need to load neighbouring map just once, not every time character moves to another overmap tile, which is what seems to be happening.

That’s pretty much the crux of the issue, from what I can tell. The overmap code in question was poorly designed, which is causing it to be way to sensitive to small differences in load time.

[pingpong]

Interesting, the bug may be related to current cursor location or which map section it’s on:

I was checking again to see if the problem persisted, and of course it did, but scrolling around I hit a patch of fast panning to the upper left of my initial shelter. If I went up from there (inside a column), it had 0 speed issues, if I went down and left, it would get worse, and take like 2-3x the lag of the typical cursor movement I was getting. The extra laggy spot seems to be all the lower left, but the faster spot is just a ~12 wide column.

[LazyCat]

That’s pretty much the crux of the issue, from what I can tell. The overmap code in question was poorly designed, which is causing it to be way to sensitive to small differences in load time.

Why are we guessing? Where is the guy who made those changes, can you ask him to come here?

If you look at the block of code I commented out to fix the lag with the map you will notice there is no any loading or saving but only use of that overload operator I was talking about.

All I need to know how was that whole thing working before, without overload operator, and the problem will most likely be fixed right away. So if you can explain, please do, or if you can summon here that guy who made those changes that would be even better.

[kevin.granade]

tl;dr arguably fixed as of commit ff22e6a. It lags occasionally now,m but not constantly like it was.

The problem was that it was being extremely dumb about it and re-loading the “adjacent” overmaps with every redraw, previously it was managing this without noticeable lag, but two changes pushed it over the edge. With the addition of a copy constructor, the code was initializing and then immediately copying an overmap with every redraw, and with the extension to the oter_t past char, we started using ifstream operators to read in the overmap instead of the previous getline-based approach. Both of these significantly slowed down the process, and redulted in extreme lag.

My solution was to pull the adjacent overmap variables into the parent function where they could remain loaded in between draw events. There is a pull request we’re still looking at that speeds up overmap save/load, and I’m taking a look at working around the copy constructor issue.

[LazyCat]

[quote=“Kevin Granade, post:8, topic:1104”]tl;dr arguably fixed as of commit ff22e6a. It lags occasionally now,m but not constantly like it was.

The problem was that it was being extremely dumb about it and re-loading the “adjacent” overmaps with every redraw, previously it was managing this without noticeable lag, but two changes pushed it over the edge. With the addition of a copy constructor, the code was initializing and then immediately copying an overmap with every redraw, and with the extension to the oter_t past char, we started using ifstream operators to read in the overmap instead of the previous getline-based approach. Both of these significantly slowed down the process, and redulted in extreme lag.[/quote]

Why do you think so, are you using a profiler or something, guessing? I placed add_msg() call where the overmap is saving and loading, it happens only once when character moves across the border to another overmap, at least in my code, and the lag is still there.

If you look at the block of code I commented out to fix the lag with the map (‘m’), you can see there is no any loading or saving, but only use of that overload operator. Can you explain how was it working before overload operator, in relation to overload operator itself, what code it replaces, why was it introduced?

[LazyCat]

This fixes the lag when moving character close to overmap boreders in the play-field:

1.) game.cpp, void game::update_map(int &x, int &y)

Move update_overmap_seen(); from the end of that function into this block of code in the same function:

[code] if(olevx != 0 || olevy != 0)
{
cur_om.save();
cur_om = overmap(this, cur_om.pos().x + olevx, cur_om.pos().y + olevy);

update_overmap_seen();
add_msg("CROSSING REGION BORDER...");

}[/code]

You could also place this bit before call to m.shift(this, levx, levy, levz, shiftx, shifty);

[code] if(!shiftx && !shifty)
return;

m.shift(this, levx, levy, levz, shiftx, shifty);

levx += shiftx;
levy += shifty;
.
.
[/code]

2.) mapgen.cpp, void map::generate(game *g, overmap *om, const int x, const int y, const int z, const int turn)

.
.
 else {
//  dbg(D_INFO) << "map::generate: In section 2";

g->add_msg("SECTION B, GENERATE-1");

  t_above = om->ter(overx, overy, z + 1);
  terrain_type = om->ter(overx, overy, z);

  if (overy - 1 >= 0)
   t_north = om->ter(overx, overy - 1, z);
  else {
//   overmap tmp(g, om->pos().x, om->pos().y - 1);
//   t_north = tmp.ter(overx, OMAPY - 1, z);
  }

  if (overx + 1 < OMAPX)
   t_east = om->ter(overx + 1, overy, z);
  else {
//   overmap tmp(g, om->pos().x + 1, om->pos().y);
//   t_east = tmp.ter(0, overy, z);
  }

  if (overy + 1 < OMAPY)
   t_south = om->ter(overx, overy + 1, z);
  else {
//   overmap tmp(g, om->pos().x, om->pos().y + 1);
//   t_south = tmp.ter(overx, 0, z);
  }

  if (overx - 1 >= 0)
   t_west = om->ter(overx - 1, overy, z);
  else {
//   overmap tmp(g, om->pos().x - 1, om->pos().y);
//   t_west = tmp.ter(OMAPX - 1, overy, z);
  }

g->add_msg("SECTION B, GENERATE-2");

 }

Strangely “generate in section 1” block of code doesn’t seem to be executed ever, even if it seems that’s the one that should be handling overmaps when crossing overmap border.

I obviously haven’t looked into it past removing the glitch, so these changes could be producing some other bugs or have who knows what side effects, but since you guys are far more familiar with the code in general that’s something you should be able to sort out better than me, and fix it properly. Please let me know when you do. I haven’t seen any bugs though, but that’s very strange fix and so there are most likely more things that need to be changed to make it proper and complete.

[Soron]

Moving update_overmap_seen() into that if statement makes it so that map tiles don’t get revealed properly when you’re exploring. That’s a pretty fatal bug, I’d say.

Returning early if shiftx and shifty are both zero sounds like it’s worth considering, but I don’t trust that there won’t be any side effects - possibly large ones. Someone would have to actually poke around and make sure it’s safe to return from there, before it could be incorporated into mainline.

In (2), I don’t trust the random removal of code without justification or explanation, especially because it looks like t_north (etc.) could end up being uninitialized (Bad Thing, obviously).

[wad67]

I have been running the most recent git build and I have found that the overmap lags significantly when you are viewing anywhere near unexplored areas.
Revealing the map stopped the lag completely and it only returned when I scrolled too close to the unexplored “#” areas/

[Soron]

Huh, now that’s interesting. It’s somewhat expected that if you reveal the overmap, you’d get more lag when you scroll near the unexplored areas - because those are other overmaps, and hence there’s additional loading that needs to happen.

However, it seems odd (to me) that lag would be higher before you reveal it… I wonder what’s causing that.

Also, wad67, just to verify: are you seeing this in a version that is before or after Kevin’s commit ff22e6a?

[LazyCat]

Moving update_overmap_seen() into that if statement makes it so that map tiles don’t get revealed properly when you’re exploring. That’s a pretty fatal bug, I’d say.

The important thing was to isolate glitch offender. I fixed it properly since then.

Returning early if shiftx and shifty are both zero sounds like it's worth considering, but I don't trust that there won't be any side effects - possibly large ones. Someone would have to actually poke around and make sure it's safe to return from there, before it could be incorporated into mainline.

It is safe, but it doesn’t make lag better.

In (2), I don't trust the random removal of code without justification or explanation, especially because it looks like t_north (etc.) could end up being uninitialized (Bad Thing, obviously).

It eliminates lag, that’s not random. What’s the justification for the code I commented out, to produce lag? Anyway, at least you now know where the bugs are, you can fix them any way you wish.

[kevin.granade]

I know the issue with scrolling aroud the overmap display was caused by excessive overmap loads because I:

1. read the code and found that the overmap constructor calls overmap::load()
2. Instrumented the code to confirm that it was calling overmap::load() excessively.
3. Hoisted the calls into the parent function and saw the symptoms disappear with no arifacts.

The code you’re commenting out without understanding it is determining what adjacent overmaps are on-screen and loading them, which means that your overmap display can no longer show features that aren’t on the current overmap. Luckily for you (or unluckily depending on your POV), Whales is a conservative coder, and the overmaps are safe to access even when uninitialized.

For the totally seperate issue of walking around near overmap borders causing brief freezes, thanks for narrowing down which code seems to be causing the issue, as you’re probably aware this kind of thing is pretty resistant to profiling.

Re: commenting out random code, you might want to take a look at that chip on your shoulder about the quality of code in this project. While the code is in a style that pretty much no one but Whales likes, and quite unmaintainable to boot, it WORKS. If you comment out code without completely understanding what it does, you ARE breaking something. This doesn’t affect the project as a whole because we aren’t going to be merging it, but your fork is going to be incredibly bug-riddled if you keep doing things like this.

[LazyCat]

[quote=“Kevin Granade, post:15, topic:1104”]I know the issue with scrolling aroud the overmap display was caused by excessive overmap loads because I:

1. read the code and found that the overmap constructor calls overmap::load()
2. Instrumented the code to confirm that it was calling overmap::load() excessively.
3. Hoisted the calls into the parent function and saw the symptoms disappear with no arifacts.

The code you’re commenting out without understanding it is determining what adjacent overmaps are on-screen and loading them, which means that your overmap display can no longer show features that aren’t on the current overmap. Luckily for you (or unluckily depending on your POV), Whales is a conservative coder, and the overmaps are safe to access even when uninitialized.

For the totally seperate issue of walking around near overmap borders causing brief freezes, thanks for narrowing down which code seems to be causing the issue, as you’re probably aware this kind of thing is pretty resistant to profiling.

Re: commenting out random code, you might want to take a look at that chip on your shoulder about the quality of code in this project. While the code is in a style that pretty much no one but Whales likes, and quite unmaintainable to boot, it WORKS. If you comment out code without completely understanding what it does, you ARE breaking something. This doesn’t affect the project as a whole because we aren’t going to be merging it, but your fork is going to be incredibly bug-riddled if you keep doing things like this.[/quote]

I said I wasn’t looking into it past isolating and removing the glitch, and was hoping you would be able to fix it properly knowing where the problem is. Since then I did fix it properly, the glitch in the play-field not just the map. Have you fixed it at all?

You only have to try it out to realize it is your code which is bug-ridden. Instead of less and less you have more and more bugs with every new release. I challenge you to find a single bug in my code.