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

VisibilityNotifier2D - very high memory usage #40577

Closed
zzz-assault opened this issue Jul 21, 2020 · 5 comments
Closed

VisibilityNotifier2D - very high memory usage #40577

zzz-assault opened this issue Jul 21, 2020 · 5 comments

Comments

@zzz-assault
Copy link
Contributor

zzz-assault commented Jul 21, 2020

Godot version:

  • 3.2.2 (stable) - 64 Bit Windows Client

OS/device including version:

  • Windows 10 (64 Bit)

Issue description:

Size of VisibilityNotifier2D Rect2 is exponentially increasing the required memory (static)!
This is true if you increase Rect2 size either within editor or via code.

Example:

  • Rect2 (0,0 // 300000, 300000) -> Memory (static) usage = 1,76 GiB

This example is extrem, but it shows the issue.
In my case I needed > 100 notifiers and even on small Rect2 sizes it scaled the memory effect very fast.

Steps to reproduce:

Add a VisibilityNotifier2D and increase Rect2 in editor or use below code.

Minimal reproduction project:

Just add following code to a new 2D Scene:

func _ready() -> void:
	var s := VisibilityNotifier2D.new()
	s.rect = Rect2(Vector2(0.0, 0.0), Vector2(300000.0, 300000.0))
	add_child(s)
@Calinou
Copy link
Member

Calinou commented Jul 21, 2020

Out of curiosity, can you reproduce this with VisibilityNotifier (3D)?

@zzz-assault
Copy link
Contributor Author

@Calinou just tried -> NO! ... VisibilityNotifier (3D) is simply staying on almost no memory usage.
Even after increasing all axis to an absurd 3.000.000.000 it had no issue.

@qarmin
Copy link
Contributor

qarmin commented Jul 22, 2020

This is probably caused by too small cell size.
You can set in Project Settings -> World -> 2d -> Cell Size bigger value e.g. 1000 and then memory usage should drop to ~40MB

@zzz-assault
Copy link
Contributor Author

zzz-assault commented Jul 22, 2020

@qarmin Thank you, will test it.

Do you know if cell_size is based on pixels? (i.e. 100 = 100x100 pixel?)

What is the downside of increasing this setting?

And why is this setting in the project settings and not directly added as property in the VisibilityNotifier2D? If the description is correct, it's only affecting the node VisibilityNotifier2D. Rather confusing for the user.

@zzz-assault
Copy link
Contributor Author

Update: I can confirm, that the memory usage with increased "cell_size" in the project settings is getting better. Also it's worth noting, that the cell_size is in PIXELs, for others looking up this issue!

For my project I set the cell_size to the full grid (Rect2), so the memory cost is almost non existant ... as all my Visibilty Notifier are having the same size, this is working also based on the global setting and the lost precision is not noticable.

Question:
I understand that heuristic is used for performance reasons, but wouldn't it be more flexible to set the cell_size inside the VisibilityNotifier2D or better directly use the dimension of the Rect2 as one Cell? ( I don't get the reason to split the notifier in multiple cells, besides flexilbility for different VisibilityNotifier2D sizes -> which could be solved by moving this setting to the Node instead of a global project setting)

@akien-mga akien-mga added archived and removed bug labels Sep 15, 2020
Calinou added a commit to Calinou/godot that referenced this issue Sep 15, 2020
akien-mga pushed a commit that referenced this issue Sep 15, 2020
MarcusElg pushed a commit to MarcusElg/godot that referenced this issue Oct 19, 2020
huhund pushed a commit to huhund/godot that referenced this issue Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants