-
-
Notifications
You must be signed in to change notification settings - Fork 23.8k
[3.x] Fix low directional shadow quality on ultrawide aspect ratios #43207
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
Open
Calinou
wants to merge
1
commit into
godotengine:3.x
Choose a base branch
from
Calinou:fix-ultrawide-directional-shadows
base: 3.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+90
−34
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2458,7 +2458,7 @@ void VisualServerScene::_update_instance_lightmap_captures(Instance *p_instance) | |
| p_instance->lightmap_capture_data.write[0].a = interior ? 0.0f : 1.0f; | ||
| } | ||
|
|
||
| bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, const Transform p_cam_transform, const CameraMatrix &p_cam_projection, bool p_cam_orthogonal, RID p_shadow_atlas, Scenario *p_scenario, uint32_t p_visible_layers) { | ||
| bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, const Transform p_cam_transform, const CameraMatrix &p_cam_projection, bool p_cam_orthogonal, bool p_cam_vaspect, RID p_shadow_atlas, Scenario *p_scenario, uint32_t p_visible_layers) { | ||
| InstanceLightData *light = static_cast<InstanceLightData *>(p_instance->base_data); | ||
|
|
||
| Transform light_transform = p_instance->transform; | ||
|
|
@@ -2559,8 +2559,6 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
|
|
||
| bool overlap = VSG::storage->light_directional_get_blend_splits(p_instance->base); | ||
|
|
||
| float first_radius = 0.0; | ||
|
|
||
| for (int i = 0; i < splits; i++) { | ||
| // setup a camera matrix for that range! | ||
| CameraMatrix camera_matrix; | ||
|
|
@@ -2572,8 +2570,8 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
|
|
||
| camera_matrix.set_orthogonal(vp_he.y * 2.0, aspect, distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], false); | ||
| } else { | ||
| float fov = p_cam_projection.get_fov(); | ||
| camera_matrix.set_perspective(fov, aspect, distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], false); | ||
| float fov = p_cam_projection.get_fov(); //this is actually yfov, because set aspect tries to keep it | ||
| camera_matrix.set_perspective(fov, aspect, distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], true); | ||
| } | ||
|
|
||
| //obtain the frustum endpoints | ||
|
|
@@ -2602,8 +2600,6 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| float z_min_cam = 0.f; | ||
| //float z_max_cam = 0.f; | ||
|
|
||
| float bias_scale = 1.0; | ||
|
|
||
| //used for culling | ||
|
|
||
| for (int j = 0; j < 8; j++) { | ||
|
|
@@ -2632,21 +2628,19 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| z_max = d_z; | ||
| } | ||
| } | ||
| float radius = 0; | ||
| Vector3 center; | ||
|
|
||
| { | ||
| //camera viewport stuff | ||
|
|
||
| Vector3 center; | ||
|
|
||
| for (int j = 0; j < 8; j++) { | ||
| center += endpoints[j]; | ||
| } | ||
| center /= 8.0; | ||
|
|
||
| //center=x_vec*(x_max-x_min)*0.5 + y_vec*(y_max-y_min)*0.5 + z_vec*(z_max-z_min)*0.5; | ||
|
|
||
| float radius = 0; | ||
|
|
||
| for (int j = 0; j < 8; j++) { | ||
| float d = center.distance_to(endpoints[j]); | ||
| if (d > radius) { | ||
|
|
@@ -2656,17 +2650,10 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
|
|
||
| radius *= texture_size / (texture_size - 2.0); //add a texel by each side | ||
|
|
||
| if (i == 0) { | ||
| first_radius = radius; | ||
| } else { | ||
| bias_scale = radius / first_radius; | ||
| } | ||
|
|
||
| x_max_cam = x_vec.dot(center) + radius; | ||
| x_min_cam = x_vec.dot(center) - radius; | ||
| y_max_cam = y_vec.dot(center) + radius; | ||
| y_min_cam = y_vec.dot(center) - radius; | ||
| //z_max_cam = z_vec.dot(center) + radius; | ||
| z_min_cam = z_vec.dot(center) - radius; | ||
|
|
||
| if (depth_range_mode == VS::LIGHT_DIRECTIONAL_SHADOW_DEPTH_RANGE_STABLE) { | ||
|
|
@@ -2703,6 +2690,7 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
|
|
||
| Plane near_plane(light_transform.origin, -light_transform.basis.get_axis(2)); | ||
|
|
||
| float cull_max = 0; | ||
| for (int j = 0; j < cull_count; j++) { | ||
| float min, max; | ||
| Instance *instance = instance_shadow_cull_result[j]; | ||
|
|
@@ -2716,11 +2704,72 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| instance->transformed_aabb.project_range_in_plane(Plane(z_vec, 0), min, max); | ||
| instance->depth = near_plane.distance_to(instance->transform.origin); | ||
| instance->depth_layer = 0; | ||
| if (max > z_max) { | ||
| z_max = max; | ||
| if (j == 0 || max > cull_max) { | ||
| cull_max = max; | ||
| } | ||
| } | ||
|
|
||
| if (cull_max > z_max) { | ||
| z_max = cull_max; | ||
| } | ||
|
|
||
| if (aspect != 1.0) { | ||
| // if the aspect is different, then the radius will become larger. | ||
| // if this happens, then bias needs to be adjusted too, as depth will increase | ||
| // to do this, compare the depth of one that would have resulted from a square frustum | ||
|
|
||
| CameraMatrix camera_matrix_square; | ||
| if (p_cam_orthogonal) { | ||
| Vector2 vp_he = camera_matrix.get_viewport_half_extents(); | ||
| if (p_cam_vaspect) { | ||
| camera_matrix_square.set_orthogonal(vp_he.x * 2.0, 1.0, distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], true); | ||
| } else { | ||
| camera_matrix_square.set_orthogonal(vp_he.y * 2.0, 1.0, distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], false); | ||
| } | ||
| } else { | ||
| Vector2 vp_he = camera_matrix.get_viewport_half_extents(); | ||
| if (p_cam_vaspect) { | ||
| camera_matrix_square.set_frustum(vp_he.x * 2.0, 1.0, Vector2(), distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], true); | ||
| } else { | ||
| camera_matrix_square.set_frustum(vp_he.y * 2.0, 1.0, Vector2(), distances[(i == 0 || !overlap) ? i : i - 1], distances[i + 1], false); | ||
| } | ||
| } | ||
|
|
||
| Vector3 endpoints_square[8]; // frustum plane endpoints | ||
| res = camera_matrix_square.get_endpoints(p_cam_transform, endpoints_square); | ||
| ERR_CONTINUE(!res); | ||
| Vector3 center_square; | ||
| float z_max_square = 0; | ||
|
|
||
| for (int j = 0; j < 8; j++) { | ||
| center_square += endpoints_square[j]; | ||
|
|
||
| float d_z = z_vec.dot(endpoints_square[j]); | ||
|
|
||
| if (j == 0 || d_z > z_max_square) | ||
| z_max_square = d_z; | ||
| } | ||
|
|
||
| if (cull_max > z_max_square) { | ||
| z_max_square = cull_max; | ||
| } | ||
|
|
||
| center_square /= 8.0; | ||
|
|
||
| float radius_square = 0; | ||
|
|
||
| for (int j = 0; j < 8; j++) { | ||
| float d = center_square.distance_to(endpoints_square[j]); | ||
| if (d > radius_square) | ||
| radius_square = d; | ||
| } | ||
|
|
||
| radius_square *= texture_size / (texture_size - 2.0); //add a texel by each side | ||
|
|
||
| // this is not entirely perfect, because the cull-adjusted z-max may be different | ||
| // but at least it's warranted that it results in a greater bias, so no acne should be present either way. | ||
| } | ||
|
|
||
| { | ||
| CameraMatrix ortho_camera; | ||
| real_t half_x = (x_max_cam - x_min_cam) * 0.5; | ||
|
|
@@ -2732,7 +2781,14 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| ortho_transform.basis = transform.basis; | ||
| ortho_transform.origin = x_vec * (x_min_cam + half_x) + y_vec * (y_min_cam + half_y) + z_vec * z_max; | ||
|
|
||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, ortho_camera, ortho_transform, 0, distances[i + 1], i, bias_scale); | ||
| VSG::scene_render->light_instance_set_shadow_transform( | ||
| light->instance, | ||
| ortho_camera, | ||
| ortho_transform, | ||
| z_max - z_min_cam, | ||
| distances[i + 1], | ||
| i, | ||
| radius * 2.0 / texture_size); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem anything like the bias scale from before (which was In master this value is |
||
| } | ||
|
|
||
| // Do a secondary cull to remove casters that don't intersect with the camera frustum. | ||
|
|
@@ -2787,7 +2843,7 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| } | ||
| } | ||
|
|
||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, CameraMatrix(), light_transform, radius, 0, i); | ||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, CameraMatrix(), light_transform, radius, 0, i, 0); | ||
| VSG::scene_render->render_shadow(light->instance, p_shadow_atlas, i, (RasterizerScene::InstanceBase **)instance_shadow_cull_result, cull_count); | ||
| } | ||
| } else { //shadow cube | ||
|
|
@@ -2843,12 +2899,12 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| } | ||
| } | ||
|
|
||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, cm, xform, radius, 0, i); | ||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, cm, xform, radius, 0, i, 0); | ||
| VSG::scene_render->render_shadow(light->instance, p_shadow_atlas, i, (RasterizerScene::InstanceBase **)instance_shadow_cull_result, cull_count); | ||
| } | ||
|
|
||
| //restore the regular DP matrix | ||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, CameraMatrix(), light_transform, radius, 0, 0); | ||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, CameraMatrix(), light_transform, radius, 0, 0, 0); | ||
| } | ||
|
|
||
| } break; | ||
|
|
@@ -2883,7 +2939,7 @@ bool VisualServerScene::_light_instance_update_shadow(Instance *p_instance, cons | |
| } | ||
| } | ||
|
|
||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, cm, light_transform, radius, 0, 0); | ||
| VSG::scene_render->light_instance_set_shadow_transform(light->instance, cm, light_transform, radius, 0, 0, 0); | ||
| VSG::scene_render->render_shadow(light->instance, p_shadow_atlas, 0, (RasterizerScene::InstanceBase **)instance_shadow_cull_result, cull_count); | ||
|
|
||
| } break; | ||
|
|
@@ -2937,7 +2993,7 @@ void VisualServerScene::render_camera(RID p_camera, RID p_scenario, Size2 p_view | |
|
|
||
| Transform camera_transform = _interpolation_data.interpolation_enabled ? camera->get_transform_interpolated() : camera->transform; | ||
|
|
||
| _prepare_scene(camera_transform, camera_matrix, ortho, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| _prepare_scene(camera_transform, camera_matrix, ortho, camera->vaspect, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| _render_scene(camera_transform, camera_matrix, 0, ortho, camera->env, p_scenario, p_shadow_atlas, RID(), -1); | ||
| #endif | ||
| } | ||
|
|
@@ -3016,17 +3072,17 @@ void VisualServerScene::render_camera(Ref<ARVRInterface> &p_interface, ARVRInter | |
| mono_transform *= apply_z_shift; | ||
|
|
||
| // now prepare our scene with our adjusted transform projection matrix | ||
| _prepare_scene(mono_transform, combined_matrix, false, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| _prepare_scene(mono_transform, combined_matrix, false, false, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| } else if (p_eye == ARVRInterface::EYE_MONO) { | ||
| // For mono render, prepare as per usual | ||
| _prepare_scene(cam_transform, camera_matrix, false, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| _prepare_scene(cam_transform, camera_matrix, false, false, camera->env, camera->visible_layers, p_scenario, p_shadow_atlas, RID(), camera->previous_room_id_hint); | ||
| } | ||
|
|
||
| // And render our scene... | ||
| _render_scene(cam_transform, camera_matrix, p_eye, false, camera->env, p_scenario, p_shadow_atlas, RID(), -1); | ||
| }; | ||
|
|
||
| void VisualServerScene::_prepare_scene(const Transform p_cam_transform, const CameraMatrix &p_cam_projection, bool p_cam_orthogonal, RID p_force_environment, uint32_t p_visible_layers, RID p_scenario, RID p_shadow_atlas, RID p_reflection_probe, int32_t &r_previous_room_id_hint) { | ||
| void VisualServerScene::_prepare_scene(const Transform p_cam_transform, const CameraMatrix &p_cam_projection, bool p_cam_orthogonal, bool p_cam_vaspect, RID p_force_environment, uint32_t p_visible_layers, RID p_scenario, RID p_shadow_atlas, RID p_reflection_probe, int32_t &r_previous_room_id_hint) { | ||
| // Prepare the light - camera volume culling system. | ||
| light_culler->prepare_camera(p_cam_transform, p_cam_projection); | ||
|
|
||
|
|
@@ -3235,7 +3291,7 @@ void VisualServerScene::_prepare_scene(const Transform p_cam_transform, const Ca | |
| VSG::scene_render->set_directional_shadow_count(directional_shadow_count); | ||
|
|
||
| for (int i = 0; i < directional_shadow_count; i++) { | ||
| _light_instance_update_shadow(lights_with_shadow[i], p_cam_transform, p_cam_projection, p_cam_orthogonal, p_shadow_atlas, scenario, p_visible_layers); | ||
| _light_instance_update_shadow(lights_with_shadow[i], p_cam_transform, p_cam_projection, p_cam_orthogonal, p_cam_vaspect, p_shadow_atlas, scenario, p_visible_layers); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -3353,7 +3409,7 @@ void VisualServerScene::_prepare_scene(const Transform p_cam_transform, const Ca | |
|
|
||
| if (redraw) { | ||
| //must redraw! | ||
| if (_light_instance_update_shadow(ins, p_cam_transform, p_cam_projection, p_cam_orthogonal, p_shadow_atlas, scenario, p_visible_layers)) { | ||
| if (_light_instance_update_shadow(ins, p_cam_transform, p_cam_projection, p_cam_orthogonal, p_cam_vaspect, p_shadow_atlas, scenario, p_visible_layers)) { | ||
| // If the light requests another update (animated material?)... | ||
| light->make_shadow_dirty(); | ||
| } | ||
|
|
@@ -3470,7 +3526,7 @@ bool VisualServerScene::_render_reflection_probe_step(Instance *p_instance, int | |
| shadow_atlas = scenario->reflection_probe_shadow_atlas; | ||
| } | ||
|
|
||
| _prepare_scene(xform, cm, false, RID(), VSG::storage->reflection_probe_get_cull_mask(p_instance->base), p_instance->scenario->self, shadow_atlas, reflection_probe->instance, reflection_probe->previous_room_id_hint); | ||
| _prepare_scene(xform, cm, false, false, RID(), VSG::storage->reflection_probe_get_cull_mask(p_instance->base), p_instance->scenario->self, shadow_atlas, reflection_probe->instance, reflection_probe->previous_room_id_hint); | ||
|
|
||
| bool async_forbidden_backup = VSG::storage->is_shader_async_hidden_forbidden(); | ||
| VSG::storage->set_shader_async_hidden_forbidden(true); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire block does nothing I think.
radius_squareis unused, insteadradiusis used later.In fact maybe the entire block from
if (aspect != 1.0)toradius_square *= ...has no side effects. 🤔In 4.x, this equivalent code seems to write directly to radius, and doesn't seem to have a special case for aspect ratio outside 1.0.