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

Use a spec constant to control whether the MultiMesh branch is used in the vertex shader. #94289

Merged
merged 1 commit into from
Jul 17, 2024
Merged

Use a spec constant to control whether the MultiMesh branch is used in the vertex shader. #94289

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
4 changes: 4 additions & 0 deletions servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,10 @@ void RenderForwardMobile::_render_list_template(RenderingDevice::DrawListID p_dr

uint32_t base_spec_constants = p_params->spec_constant_base_flags;

if (bool(inst->flags_cache & INSTANCE_DATA_FLAG_MULTIMESH)) {
base_spec_constants |= 1 << SPEC_CONSTANT_IS_MULTIMESH;
}

SceneState::PushConstant push_constant;
push_constant.base_index = i + p_params->element_offset;

Expand Down
1 change: 1 addition & 0 deletions servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class RenderForwardMobile : public RendererSceneRenderRD {
SPEC_CONSTANT_DISABLE_DECALS = 13,
SPEC_CONSTANT_DISABLE_FOG = 14,
SPEC_CONSTANT_USE_DEPTH_FOG = 16,
SPEC_CONSTANT_IS_MULTIMESH = 17,

};

Expand Down
10 changes: 6 additions & 4 deletions servers/rendering/renderer_rd/shaders/forward_mobile/scene_forward_mobile.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ void axis_angle_to_tbn(vec3 axis, float angle, out vec3 tangent, out vec3 binorm
normal = omc_axis.zzz * axis + vec3(-s_axis.y, s_axis.x, c);
}

/* Spec Constants */

layout(constant_id = 17) const bool sc_is_multimesh = false;

/* Varyings */

layout(location = 0) highp out vec3 vertex_interp;
Expand Down Expand Up @@ -178,8 +182,6 @@ void main() {
color_interp = color_attrib;
#endif

bool is_multimesh = bool(instances.data[draw_call.instance_index].flags & INSTANCE_FLAGS_MULTIMESH);

Copy link
Contributor

@MarianoGnu MarianoGnu Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is changing the way is_multimesh is detected in the scene_forward_movine.glsl shader, but non mobile shaders will continue using the old way. Is there a reason to not unify criterias in different shaders?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we will be changing how multimesh are handled in both shaders in 4.4. But before we do that, we need to merge #90400 otherwise we will make the shader compilation stutter much worse.

This PR is a band aid fix for 4.3. It only targets Mobile since the Quest3 is the only device with this driver bug and the cost of adding a new spec_constant in the Forward+ shader is much higher. The way the Mobile renderer works right now, this change won't increase shader compilation stutter, so it is safe to make. In the Forward+ renderer this change could increase shader compilation stutter (its already a much bigger problem there) and since the Forward+ renderer can't be used on the Quest3, it doesn't really provide a benefit.

mat4 model_matrix = instances.data[draw_call.instance_index].transform;
mat4 inv_view_matrix = scene_data.inv_view_matrix;
#ifdef USE_DOUBLE_PRECISION
Expand All @@ -203,7 +205,7 @@ void main() {
mat4 matrix;
mat4 read_model_matrix = model_matrix;

if (is_multimesh) {
if (sc_is_multimesh) {
//multimesh, instances are for it

#ifdef USE_PARTICLE_TRAILS
Expand Down Expand Up @@ -399,7 +401,7 @@ void main() {
// Then we combine the translations from the model matrix and the view matrix using emulated doubles.
// We add the result to the vertex and ignore the final lost precision.
vec3 model_origin = model_matrix[3].xyz;
if (is_multimesh) {
if (sc_is_multimesh) {
vertex = mat3(matrix) * vertex;
model_origin = double_add_vec3(model_origin, model_precision, matrix[3].xyz, vec3(0.0), model_precision);
}
Expand Down
Loading