[Bug]: Templates crash Maptool on Gridless maps

[irisiflimsi]

Describe the Bug

When you try to use a template on a map, which has no grid, Maptool crashes usually. This effects cascades from client to server and to all other clients. All connected maptool instances need to be killed through OS means.
It looks like the template pick the pixels as gridsize, which probably makes internal calculations take forever.

To Reproduce

1. Take default map
2. edit it to have no cell type (radio button "x")
3. Select template, zoom out a little.
4. Click into the map and move the mouse a bit
5. Maptool hangs

Expected Behaviour

Probably reject templating, but at least not a crash.

Screenshots

No response

MapTool Info

1.13.2 and 1.14.3

Desktop

Linux + Windows

Additional Context

No response

[FullBleed]

Due to a recent PR to remove the max size limitation of blast templates (#5060) I checked how that template currently interacts with MT on gridless maps... and, sure enough, it doesn't crash MT like the other templates because the max setting seems to be what keep it from being created too large. Without the max setting it'll probably exhibit the same behaviour.

[kwvanderlinde]

Due to a recent PR to remove the max size limitation of blast templates (#5060) I checked how that template currently interacts with MT on gridless maps... and, sure enough, it doesn't crash MT like the other templates because the max setting seems to be what keep it from being created too large. Without the max setting it'll probably exhibit the same behaviour.

I don't believe so. It's basically the same as the burst template, just anchored at a different point. You can draw a gian burst template today and it works fine. And just to be sure, I tried out blast templates with the latest changes and there's no issue.

[kwvanderlinde]

There's two underlying problems the conflate to produce this bug.

First is that gridless maps have a different relationship between distances measured in cell units vs those measure in map units. For all other grid types, scaling by the grid size is enough to convert between the two. But for gridless maps, cell units and map units are identical despite the grid size not being 1, so such scaling does not work for them. This is the cause of templates being much larger than they should be on gridless maps. But after looking at where we use the grid size internally, I think it impacts several other things as well.

Second is template rendering performance, specifically for the radius and cone templates. These scale very poorly as the number of squares grow, so large templates of these types are just not feasible to render. This is extra noticable on gridless maps because there are so many more squares to render. I believe this poor performance is due to the strategy we use for rendering: every frame we rebuild the template as a patchwork of internal areas and various border cases, one for each square in the template. But I expect we could instead build a regular-old-Shape and render that by normal means, and we should see a vast improvement for large templates. Caveat: I haven't actually tried it, it's just intuition.

[FullBleed]

I don't believe so. It's basically the same as the burst template, just anchored at a different point. You can draw a gian burst template today and it works fine. And just to be sure, I tried out blast templates with the latest changes and there's no issue.

Sounds like you have a handle on the actual issue... I just noticed when I tried using a blast now on gridless it seemed to automatically go to the full (current) "500 max" and stop because I was zoomed so far out... whereas radius and cone templates were gigantic with even a tiny movement of the mouse and locked things up. But you're right, the line and square templates seem fine (even at ridiculous sizes.)

So the problem really seems to be limited to radius and cone. If you don't math the problem out, maybe they need a max size.

Edit: After testing out the effect of sizes on radius and cone, you could probably just go 500 max on them and call it the day. ;) Not a fix to the underlying problem, but will stop this from crashing most systems.