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.

Sign up for GitHub

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

Make lights_per_object configurable #43606

Merged
merged 1 commit into from
Nov 17, 2020
Merged

Make lights_per_object configurable #43606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/classes/ProjectSettings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,9 @@
<member name="rendering/limits/rendering/max_renderable_reflections" type="int" setter="" getter="" default="1024">
Max number of reflection probes renderable in a frame. If more than this number are used, they will be ignored. On some systems (particularly web) setting this number as low as possible can increase the speed of shader compilation.
</member>
<member name="rendering/limits/rendering/max_lights_per_object" type="int" setter="" getter="" default="32">
Max number of lights renderable per object. This is further limited by hardware support. Most devices only support 409 lights, while many devices (especially mobile) only support 102. Setting this low will slightly reduce memory usage and may decrease shader compile times.
</member>
<member name="rendering/limits/time/time_rollover_secs" type="float" setter="" getter="" default="3600">
Shaders have a time variable that constantly increases. At some point, it needs to be rolled back to zero to avoid precision errors on shader animations. This setting specifies when (in seconds).
</member>
Expand Down
11 changes: 6 additions & 5 deletions drivers/gles3/rasterizer_scene_gles3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1834,15 +1834,14 @@ void RasterizerSceneGLES3::_render_geometry(RenderList::Element *e) {

void RasterizerSceneGLES3::_setup_light(RenderList::Element *e, const Transform &p_view_transform) {

int omni_indices[16];
int maxobj = state.max_forward_lights_per_object;
int *omni_indices = (int *)alloca(maxobj * sizeof(int));
Copy link
Member

Choose a reason for hiding this comment

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

MSVC doesn't seem to like alloca here:

D:\a\godot\godot\drivers\gles3\rasterizer_scene_gles3.cpp(2219) : error C2220: the following warning is treated as an error
D:\a\godot\godot\drivers\gles3\rasterizer_scene_gles3.cpp(2219) : warning C4750: 'void __cdecl RasterizerSceneGLES3::_setup_light(struct RasterizerSceneGLES3::RenderList::Element * __ptr64,class Transform const & __ptr64) __ptr64': function with _alloca() inlined into a loop

Seems like you might have to remove _FORCE_INLINE_ for it to compile.

BTW, would it make sense to use something smaller than int to allocate less memory (e.g. uint8_t or uint16_t)? Or it doesn't matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would use less memory but even with 1000 lights it only uses around 0.005mb, so I don't really think it matters either way

Copy link
Member

Choose a reason for hiding this comment

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

int is probably fine here as the max size is 1024 * sizeof (int), but in general when using the stack, akien is absolutely right, because the stack can be of very limited size, platform dependent, and how much room you have 'depends'. It absolutely is not the same as general memory.

See e.g. here, suggesting historical figures of 8K being possible on old mobile:
https://stackoverflow.com/questions/1859327/android-stack-size

So 1024 * 8 byte int = 8K, blown stack. Whereas 1024 * 2 byte uint16_t = slightly safer, but still likely to blow such a small stack.

More realistically, modern times I would expect around a 1 meg+, especially on desktop:
https://pspdfkit.com/blog/2019/android-large-memory-requirements/

But this is not just for your function.

In short, alloca is great but make sure you know what you are doing, and beware of anything that might be called recursively. This kind of thing is why the compiler refuses to compile with a FORCE_INLINE.

int omni_count = 0;
int spot_indices[16];
int *spot_indices = (int *)alloca(maxobj * sizeof(int));
int spot_count = 0;
int reflection_indices[16];
int reflection_count = 0;

int maxobj = MIN(16, state.max_forward_lights_per_object);

int lc = e->instance->light_instances.size();
if (lc) {

Expand Down Expand Up @@ -5054,6 +5053,8 @@ void RasterizerSceneGLES3::initialize() {
ProjectSettings::get_singleton()->set_custom_property_info("rendering/limits/rendering/max_renderable_lights", PropertyInfo(Variant::INT, "rendering/limits/rendering/max_renderable_lights", PROPERTY_HINT_RANGE, "16,4096,1"));
render_list.max_reflections = GLOBAL_DEF("rendering/limits/rendering/max_renderable_reflections", (int)RenderList::DEFAULT_MAX_REFLECTIONS);
ProjectSettings::get_singleton()->set_custom_property_info("rendering/limits/rendering/max_renderable_reflections", PropertyInfo(Variant::INT, "rendering/limits/rendering/max_renderable_reflections", PROPERTY_HINT_RANGE, "8,1024,1"));
render_list.max_lights_per_object = GLOBAL_DEF_RST("rendering/limits/rendering/max_lights_per_object", (int)RenderList::DEFAULT_MAX_LIGHTS_PER_OBJECT);
ProjectSettings::get_singleton()->set_custom_property_info("rendering/limits/rendering/max_lights_per_object", PropertyInfo(Variant::INT, "rendering/limits/rendering/max_lights_per_object", PROPERTY_HINT_RANGE, "8,1024,1"));

{
//quad buffers
Expand Down Expand Up @@ -5168,7 +5169,7 @@ void RasterizerSceneGLES3::initialize() {
glBufferData(GL_UNIFORM_BUFFER, sizeof(LightDataUBO), NULL, GL_DYNAMIC_DRAW);
glBindBuffer(GL_UNIFORM_BUFFER, 0);

state.max_forward_lights_per_object = 8;
state.max_forward_lights_per_object = MIN(state.max_ubo_lights, render_list.max_lights_per_object);

state.scene_shader.add_custom_define("#define MAX_LIGHT_DATA_STRUCTS " + itos(state.max_ubo_lights) + "\n");
state.scene_shader.add_custom_define("#define MAX_FORWARD_LIGHTS " + itos(state.max_forward_lights_per_object) + "\n");
Expand Down
4 changes: 3 additions & 1 deletion drivers/gles3/rasterizer_scene_gles3.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,7 @@ class RasterizerSceneGLES3 : public RasterizerScene {
MAX_DIRECTIONAL_LIGHTS = 16,
DEFAULT_MAX_LIGHTS = 4096,
DEFAULT_MAX_REFLECTIONS = 1024,
DEFAULT_MAX_LIGHTS_PER_OBJECT = 32,

SORT_KEY_PRIORITY_SHIFT = 56,
SORT_KEY_PRIORITY_MASK = 0xFF,
Expand Down Expand Up @@ -704,6 +705,7 @@ class RasterizerSceneGLES3 : public RasterizerScene {
int max_elements;
int max_lights;
int max_reflections;
int max_lights_per_object;

struct Element {

Expand Down Expand Up @@ -836,7 +838,7 @@ class RasterizerSceneGLES3 : public RasterizerScene {
_FORCE_INLINE_ bool _setup_material(RasterizerStorageGLES3::Material *p_material, bool p_depth_pass, bool p_alpha_pass);
_FORCE_INLINE_ void _setup_geometry(RenderList::Element *e, const Transform &p_view_transform);
_FORCE_INLINE_ void _render_geometry(RenderList::Element *e);
_FORCE_INLINE_ void _setup_light(RenderList::Element *e, const Transform &p_view_transform);
void _setup_light(RenderList::Element *e, const Transform &p_view_transform);

void _render_list(RenderList::Element **p_elements, int p_element_count, const Transform &p_view_transform, const CameraMatrix &p_projection, RasterizerStorageGLES3::Sky *p_sky, bool p_reverse_cull, bool p_alpha_pass, bool p_shadow, bool p_directional_add, bool p_directional_shadows);

Expand Down