daleblackwood / goshapes

Intuitive path-based level editing for Godot 4
MIT License
156 stars 16 forks source link

Feature Request: Do not rebuild parts of a `WallMeshShaper` that are _guaranteed_ to not have changed since last update #43

Closed GGAlanSmithee closed 1 month ago

GGAlanSmithee commented 2 months ago

Feature Request: Do not rebuild parts of a WallMeshShaper that are guaranteed to not have changed since last update

This request is a bit speculative, but it should be possible to greatly improve performance when building a WallMeshShaper by simply skipping re-building parts of the wall that are guaranteed to be unchanged, allow me to explain:

There are four things that can trigger a change:

  1. a property is changed (ie. shaper.mesh, shaper.closed, options.rounding, options.interpolate)
  2. a curve point is added
  3. a curve point is removed
  4. a curve point is moved

Case 1: A property is changed

This requires the entire shape to be re-built

Case 2, 3, and 4: A curve point is added, removed, or moved

The shape needs to be re-built between the neighbouring point(s) of the added, removed, or changed point

it could be that we need to rebuild the shape for next-to-neighbours as well, in extreme cases, but it should not be needed beyond that point

Example / Illustration

Consider the extreme example below:

image image

GGAlanSmithee commented 2 months ago

hmm, I might be mistaken on this, given that altering a point of the curve/path will invalidate all subsequent segments, since the placement of the mesh segments are dependent on the previous 🤔

This greatly reduces the usefulness of this for some cases, with a time complexity of O(n) but Omega(1+1), or in other words:

daleblackwood commented 1 month ago

The present wrapping technique has pretty hefty flow-on effects, like you mentioned, so any alterations would need to continue to continue for the rest of the path, meaning compute time would depend on the region you're working on.

At the moment, I'm concentrating on getting the threaded workflow working so that more complex shapes don't interrupt working or pause the editor - there are still options on the table, like deferring collider building until after that's done. The trade-offs are difficult to manage. I'd suggest trying the multi-threaded branch for now.

GGAlanSmithee commented 1 month ago

@daleblackwood I totally understand your reasoning. I have been playing around with the multi-threaded branch since yesterday, and while it is an improvement in the sense that it does not block you from doing other things while a path is re-building, it's still not great devx-vise.

please note that I understand that what I am trying to use this addon for might not be typical, idiomatic, or even something you want to support, so I also understand that any feedback/suggestion I give might be misaligned with your goals

With the above clause in mind: I am building quite large shapes, and having to wait 10 sec to see the result when moving a point is just too much of a break in the feedback cycle to really be something I could work with in the long run. This is, ofc, only for WallMeshShapers that are sufficiently large, so everything other than that is working great, but the WallMeshShaper is such a killer feature IMO - being able to build semi organic / synthetic structures that follows a landscape is just great.

This is the reason why I am looking for alternative solutions, and why the placeholder_mesh was such a good solution (devx vise) for me, even though I totally understand why you don't like that solution on a technical plan - I too do not like fixing symptoms. I also recognize that there are work-arounds, such as breaking up the wall into multiple shapes, but I would like to avoid that, if possible.

Anyhow, to make a long story short; I will continue to test the multi-threaded branch and try to provide meaningful feedback.

https://github.com/user-attachments/assets/fa09a281-d9a6-4c74-9728-2e228f186b78

https://github.com/user-attachments/assets/ed93b125-aa95-4954-a046-f97e794791da

GGAlanSmithee commented 1 month ago

Hijacking my own issue.

My previous feedback, while valid for that specific case, is just that - very specific. I am playing around with some cave walls, and it is possible to have a great workflow with a few things in mind:

  1. Use a reasonbly low-poly mesh
  2. while modifying the shape, turn off build collision (this can be re-activated when done, if needed)
  3. use interpolate=1 while modifying the shape (can also be increased later)

This addon is amazing @daleblackwood I hope you don't mistake my feedback and discussions as being too negative, because that is not my intention.

https://github.com/user-attachments/assets/9b13bce8-877b-4c3e-b942-52c5713711b7

daleblackwood commented 1 month ago

Thanks for the kind words! Wow, yeah, I can see the performance issues there are a real problem. Would you mind sharing a sample scene with me that has a similar complexity? I'd love to take a look, see whether I can optimise the build system to make it easier to use for complex scenes.

I've just done a revision on the thread system in the 1.3 branch and I think that, with a little work, I might be able to batch-defer colliders in the system itself. I noticed too in the video that you're getting the bug where the path data lags one update behind - I think that's fixed now.

You're absolutely right about collider generation being the major issue on large meshes. I'm beginning to see that your placeholder idea may be a good option. It might also be useful to allow for a separate collider mesh. At the moment, the generator uses the built mesh on both the display and collider (unless simple collider is set, which just uses a flat wall I think) - it might be worth the extra processing upfront to have lower resolution colliders.

GGAlanSmithee commented 1 month ago

@daleblackwood as you mention in the documentation, it is possible to save the different Shapers as resources, and then you can switch the meshes there, effectively enabling using placeholder meshes by switching to a low poly mesh while editing, so it's technically possibly to work with placeholders without adding that specifically to the shaper.

Here are some platforms btw, really loving the workflow using goshapes. (PS. it would be great to be able to stich the edges together. The screenshot below uses interpolate=4, but there are still some minor visible gaps)

image

GGAlanSmithee commented 1 month ago

Here's one with a fence, using the new gaps option

image

GGAlanSmithee commented 1 month ago

To actually answer your questions though;

Also, your idea regarding deferring collision creation sounds great!

GGAlanSmithee commented 1 month ago

I pushed an extreme example to https://github.com/EdenEver/goshapes/tree/example/wall (branch example/wall)

This adds a long wall along the edges of the platforms, which is quite an extreme case :) BTW, there seems to be some potential issue with rounding (that I'm quite sure did not exist before) which is that the shape now always connects the last and first point (even without the closed option) when you have rounding turned on. Also, if you look at the wall in the example scene (platforms.tscn) there seems to be some artifacts along the path, with visual gaps, which might be a more visual example of what I mentioned a few posts ago, the last screenshot highlights this.

image image image

daleblackwood commented 1 month ago

Yes, rounding and gaps are unfortunately pretty tricky to get working together properly, I've had to change the way I calculate mesh lengths to account for gaps, though I have a few ideas about how I might be able to treat this. Thanks for posting your example, I'll have a look at that today.

daleblackwood commented 1 month ago

I have found a way to do this, I've created a WallMeshSegmentShaper in v1.3.1 which drastically improves performance using caching, selecting building and level of details. There's also gaps and the ability to place an instance at the gaps.

GGAlanSmithee commented 1 month ago

This is amazing @daleblackwood I will give this a try after work today 🙇🏼