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

selecting terrain brush on wrong layer can have massive consequences #1632

Closed
Bobjt opened this issue Jun 28, 2017 · 8 comments
Closed

selecting terrain brush on wrong layer can have massive consequences #1632

Bobjt opened this issue Jun 28, 2017 · 8 comments
Milestone

Comments

@Bobjt
Copy link

Bobjt commented Jun 28, 2017

Tiled doesn't freeze but hangs the main thread for quite some time if I select the wrong layer in conjunction with the terrains brush. Stack trace attached (though it may not be needed).
Uploading Spindump-062817-terrain-brush.txt…

Steps to reproduce:

  • create a 300 x 300 map (or, create a new layer in an existing map)
  • select the terrain brush + a terrain and move mouse over the empty layer
  • witness hang for about a minute

The hang is avoided if I first fill the new layer with a tile that exists in the selected terrain. I didn't search for existing issues, so apologies if this is a duplicate report.

@Bobjt
Copy link
Author

Bobjt commented Jun 29, 2017

Re-uploading and including a spindump from a permanent lock.

Spindump-062817-terrain-lock-this-one-is-a-permanent-hang.txt

Spindump-062817-terrain-brush.txt

I'll try to get a repro' case posted here.

@Bobjt
Copy link
Author

Bobjt commented Jun 29, 2017

It looks like the terrain logic is doing some weird stuff.

Here's a capture of the eventual preview from a long hang:
screen shot 2017-06-28 at 11 45 50 pm 2

And the map without the terrain tool preview looks like this:
screen shot 2017-06-29 at 12 13 59 pm

It was still hung however after this preview was rendered (maybe interactively hanging for long periods, if that makes any sense)

@Bobjt
Copy link
Author

Bobjt commented Jun 29, 2017

Steps to reproduce:

  1. open the attached overworld-draft-200.tmx
  • the external tileset it depends on is in the "_tilesets.zip" bundle - it's the addlsheet1.tsx
  • the graphics that addlsheet1.tsx depends on is attached. Unzip: addlsheet1.png.zip
  1. in Tiled, select the layer "Layer 0 a" (this is the layer on which all tiles are currently defined. All tiles on this layer are member of the various terrain definitions (even the clear portions). You'll see those after opening the terrains window on this map.

  2. select the terrain definition "oworld1 mountains" and bring the cursor over to the center of the map.

  3. experience the hang. That's all.

addlsheet1.png.zip
overworld-draft-200.tmx.zip
_tilesets.zip

@bjorn
Copy link
Member

bjorn commented Jul 4, 2017

So, this is due to a combination of factors:

  • Your "oworld1 mountains" terrain only has transitions to "nothing". In your tileset there does not exist any way to connect "oworld1 mountains" to the terrains used by the existing tiles on the "Layer 0 a". This causes it to change the whole map as it futilely tries to fix up the border, which keeps expanding to fill the whole map (which is relatively large). Using "oworld1 mountains" on an empty layer works fine, because the border is immediately fine.

  • Your tileset contains a huge amount of tiles (65536). This is part of the problem because at the moment the terrain tool uses a linear search through the tiles in the tileset to find one that matches a certain terrain criteria. A workaround is to separate your terrain tiles into a smaller tileset (though that can only make it about 100x faster).

The solution would be:

  • Improve the lookup of tiles to not be a linear search through the whole tileset.

  • Improve the algorithm filling the map, such that it searches for the smallest modification and will give up after some time.

@bjorn
Copy link
Member

bjorn commented Jul 4, 2017

This issue is something to keep an eye on in relation to Wang Tiles (#1455), which @Bdtrotte is working on. Wang Tiles may replace the terrain tool as a more flexible solution and we could make sure that the approach does not suffer from the above issues.

@Bdtrotte
Copy link
Contributor

Bdtrotte commented Jul 4, 2017

Yeah, I think so too. I haven't thought this far ahead yet... but hopefully finding the transitions will be much faster with the hash maps already in place. I imagine the eventual "wang brush" will be able to match the functionality of the terrain brush

bjorn added a commit that referenced this issue Jul 25, 2017
This is mainly useful for making the brush work on infinite maps, but in
general it is good to prevent the tool from going crazy when it doesn't
have a complete set of transition tiles.

The area modified by the tool is now limitd to the maximum distance
between any two terrains, as counted by the number of transitions
needed.

A 'checked' boolean was added to each Cell so that the terrain tool can
use the chunked allocation logic of TileLayer, while still having a
fast place to store whether it processed a certain location or not.

This helps a lot with issue #1632
@bjorn
Copy link
Member

bjorn commented Jul 25, 2017

While making the Terrain Brush work for infinite maps, I had to apply some kind of limit to the area affected by the brush. This also helped a great deal in resolving this issue, though the wait is still considerable. It is only processing for about 5 seconds on my machine after the above change, which is definitely less fatal.

bjorn added a commit that referenced this issue Jul 25, 2017
This is mainly useful for making the brush work on infinite maps, but in
general it is good to prevent the tool from going crazy when it doesn't
have a complete set of transition tiles.

The area modified by the tool is now limitd to the maximum distance
between any two terrains, as counted by the number of transitions
needed.

A 'checked' boolean was added to each Cell so that the terrain tool can
use the chunked allocation logic of TileLayer, while still having a
fast place to store whether it processed a certain location or not.

This helps a lot with issue #1632
ketanhwr pushed a commit to ketanhwr/tiled that referenced this issue Jul 28, 2017
This is mainly useful for making the brush work on infinite maps, but in
general it is good to prevent the tool from going crazy when it doesn't
have a complete set of transition tiles.

The area modified by the tool is now limitd to the maximum distance
between any two terrains, as counted by the number of transitions
needed.

A 'checked' boolean was added to each Cell so that the terrain tool can
use the chunked allocation logic of TileLayer, while still having a
fast place to store whether it processed a certain location or not.

This helps a lot with issue mapeditor#1632
@bjorn bjorn added this to the Tiled 1.5 milestone Sep 29, 2020
@bjorn
Copy link
Member

bjorn commented Oct 8, 2020

With the Wang Brush now improved such that it could entirely replace the Terrain Brush (see #2888) I think we have also resolved this issue. This is because in addition to limiting the area it is changing, it only searches those tiles which have Wang colors assigned, rather than iterating the whole tileset.

I've opened the example attached in #1632 (comment) and while it still can't do anything useful with "oworld1 mountains", it's now very responsive. It's also fast while editing with any of the more properly set up terrains.

I hope it still helps you, @Bobjt, but in any case thank you again for your amazing support of Tiled!

@bjorn bjorn closed this as completed Oct 8, 2020
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

No branches or pull requests

3 participants