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

A more efficient way of terrain details processing #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

painfulexistence
Copy link

@painfulexistence painfulexistence commented Jan 15, 2022

  1. Maintain a dict of AABBs for all detail chunks

  2. Update all chunk AABBs only when necessary:

  • On terrain setup
  • On terrain transform changes
  • On terrain chunks rebuild, which happens when terrain data resolution changes
  1. The number of times of get_region_aabb func calls is now independent of the number of detail layers

addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detail_layer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detail_layer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detail_layer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
addons/zylann.hterrain/hterrain_detailer.gd Outdated Show resolved Hide resolved
@TokisanGames
Copy link
Contributor

Nice work @painfulexistence . I did some testing on it.

  • Win10/64 GTX1060
  • 2k terrain, with only a player, directional light, and an hdr
  • Three detail layers: 2D grass plane, 3D grass mesh, 3D rock.

Results w/ 3 Detail Layers

FPS

  • Before: 40-58fps. It seemed fairly consistent, only a few times peaking at 58, mostly around 45-55.
  • After: 38-60fps. It seems to swing a bit more, achieving both higher and lower results.

Script Time

  • Before
    image

  • After
    image

Full Scene

w/ day/night cycle, water, trees, animals, houses, etc.

  • Before: 28-45fps
  • After: 28-45fps

8 detail layers

Here I added more 2D textures and 3D meshes for flowers, pinecones and branches.

  • Before: 27-38fps
    image

  • After: 24-39fps
    image

Overall, it seems the FPS performance of the old classes is slightly more consistent. I can't tell if the FPS is any faster. We must be bound by the performance of the renderer and MMI. However there is a definite improvement to script time.

I think for our game, we're just going to have to limit the use of grass with sparse ground vegetation so as to not overwhelm the renderer.

@painfulexistence painfulexistence changed the title A more efficient way of terrain details processing Draft - A more efficient way of terrain details processing Jan 17, 2022
@painfulexistence painfulexistence force-pushed the terrain_detailer branch 2 times, most recently from 9d20b5a to 324ca4f Compare February 5, 2022 06:25
@painfulexistence painfulexistence changed the title Draft - A more efficient way of terrain details processing A more efficient way of terrain details processing Feb 5, 2022
Copy link
Owner

@Zylann Zylann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like some things can still be optimized by applying coordinate conversions differently.
Also, I think you are still missing the case where the terrain is sculpted.

@@ -81,6 +80,8 @@ var _debug_wirecube_mesh: Mesh = null
var _debug_cubes := []
var _logger := Logger.get_for(self)

var _chunk_size: int = load("res://addons/zylann.hterrain/hterrain_detailer.gd").CHUNK_SIZE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the constant could actually be in that script instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean define CHUNK_SIZE in both script, or define it in the detail_layer.gd and then reference it from other scripts if needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zylann I updated it as putting CHUNK_SIZE in both script. Is this okay with you?
If so, then this PR is ready to be merged🙏

addons/zylann.hterrain/hterrain_detail_layer.gd Outdated Show resolved Hide resolved
@painfulexistence
Copy link
Author

Hi @Zylann,
Sorry if this is a dumb question.
But would you mind pointing out to me which signal I can listen to for knowing the terrain being sculpted?
I'd tried to find that signal and put a line to recalculate AABBs as the comment above, but didn't know what else I could do.

@Zylann
Copy link
Owner

Zylann commented Feb 13, 2022

There is the following signal in HTerrainData:

signal region_changed(x, y, w, h, channel)

However it is emitted for other channels than just the heightmap.
There is a function in hterrain.gd from where you can call the detailer directly, _on_data_region_changed.
Note, this is called everytime the user sculpts, so you will need to implement AABB updates with a region instead of updating them all.

I still have a feeling that extra cache is really just complexity inflicted by GDScript alone, because bounds precomputed inside HTerrainData should have been fast enough to use...
I'm also wondering if chunking should also be done via a sliding box algorithm instead of iterating all cells everytime. The idea is to convert viewer coordinates into chunk coordinates, and detect when they change (which is not often). And only then, load chunks only in the new cells, and unload chunks only in the cells out of range. That is a lot less iterations to do. Only downside would be the area would be square instead of circular, so a few chunks would be wasted (due to the shader they would not be visible and yet still drawn).

- Update detail chunk AABBs only when necessary
- Maintain a list of calculated AABBs for subsequent frames
@painfulexistence
Copy link
Author

@Zylann
I've updated the PR to implement AABB updates for a changed region.

Yes, I understand the extra cache of AABBs is somewhat redundant, but I could't come up with other ways to make detail chunk AABBs update faster😭
The sliding box optimization sounds interesting to me, though I need some time to figure it out and I'm kinda busy recently.
Is it okay that I leave that for the future updates?

@Zylann
Copy link
Owner

Zylann commented Feb 17, 2022

Yeah sliding box could be done later

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

Successfully merging this pull request may close these issues.

3 participants