RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-26 08:27:45 +00:00
Code Issues Activity Actions

LibSoftGPU: Use non-normalized light vector for attenuation

Browse source 
There were some issues with the old code: we were saving the length of
the light vector but not actually using it anywhere, if we were dealing
with a zero-vector this could potentially divide by zero resulting in a
black fragment color, and we were erroneously using P2's length instead
of P1's length when P1's W coordinate is zero in the SGI arrow
operation.

This fixes some lighting bugs in Grim Fandango, but this probably
affects all lighting as well.
This commit is contained in:
Jelle Raaijmakers 2022-03-03 11:33:29 +01:00 • committed by Andreas Kling
parent 743c0f0882
commit e1c863d99a
1 changed files with 20 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

51
Userland/Libraries/LibSoftGPU/Device.cpp
View file

@ -803,42 +803,33 @@ void Device::draw_primitives(PrimitiveType primitive_type, FloatMatrix4x4 const&
if (!light.is_enabled) if (!light.is_enabled)
continue; continue;
// We need to save the length here because the attenuation factor requires a non // We need to save the length here because the attenuation factor requires a non-normalized vector!
// normalized vector! auto sgi_arrow_operator = [](FloatVector4 const& p1, FloatVector4 const& p2, float& output_length) {
auto sgi_arrow_operator = [](FloatVector4 const& p1, FloatVector4 const& p2, float& saved_length) { FloatVector3 light_vector;
if ((p1.w() != 0.0f) && (p2.w() == 0.0f)) { if ((p1.w() != 0.f) && (p2.w() == 0.f))
saved_length = p2.length(); light_vector = p2.xyz();
return (p2 / saved_length).xyz(); else if ((p1.w() == 0.f) && (p2.w() != 0.f))
} else if ((p1.w() == 0.0f) && (p2.w() != 0.0f)) { light_vector = -p1.xyz();
saved_length = p2.length(); else
return -(p1 / saved_length).xyz(); light_vector = p2.xyz() - p1.xyz();
} else {
// FIXME: The OpenGL 1.5 spec says nothing about the case where P1 and P2 BOTH have a w value of 1, which would output_length = light_vector.length();
// then mean the light position has an implicit value of (0, 0, 0, 0). This doesn't make any logical sense, and it most likely if (output_length == 0.f)
// a typographical error. Most other GL implementations seem to just fix it to the distance from the vertex to the light, which return light_vector;
// seems to work just fine. return light_vector / output_length;
// If somebody with more insight about this could clarify this eventually, that'd be great.
auto distance = (p2 - p1);
saved_length = distance.length();
return (distance / saved_length).xyz();
}
}; };
auto sgi_dot_operator = [](FloatVector3 const& d1, FloatVector3 const& d2) { auto sgi_dot_operator = [](FloatVector3 const& d1, FloatVector3 const& d2) {
return AK::max(d1.dot(d2), 0.0f); return AK::max(d1.dot(d2), 0.0f);
}; };
float vector_length = 0.0f; float vertex_to_light_length = 0.f;
FloatVector3 vertex_to_light = sgi_arrow_operator(vertex.eye_coordinates, light.position, vector_length); FloatVector3 vertex_to_light = sgi_arrow_operator(vertex.eye_coordinates, light.position, vertex_to_light_length);
// Light attenuation value. // Light attenuation value.
float light_attenuation_factor = 1.0f; float light_attenuation_factor = 1.0f;
if (light.position.w() != 0.0f) { if (light.position.w() != 0.0f)
auto const vertex_to_light_length = vertex_to_light.length(); light_attenuation_factor = 1.0f / (light.constant_attenuation + (light.linear_attenuation * vertex_to_light_length) + (light.quadratic_attenuation * vertex_to_light_length * vertex_to_light_length));
auto const vertex_to_light_length_squared = vertex_to_light_length * vertex_to_light_length;
light_attenuation_factor = 1.0f / (light.constant_attenuation + (light.linear_attenuation * vertex_to_light_length) + (light.quadratic_attenuation * vertex_to_light_length_squared));
}
// Spotlight factor // Spotlight factor
float spotlight_factor = 1.0f; float spotlight_factor = 1.0f;
@ -872,7 +863,7 @@ void Device::draw_primitives(PrimitiveType primitive_type, FloatMatrix4x4 const&
if (!m_lighting_model.viewer_at_infinity) { if (!m_lighting_model.viewer_at_infinity) {
half_vector_normalized = (vertex_to_light + FloatVector3(0.0f, 0.0f, 1.0f)).normalized(); half_vector_normalized = (vertex_to_light + FloatVector3(0.0f, 0.0f, 1.0f)).normalized();
} else { } else {
auto const vertex_to_eye_point = sgi_arrow_operator(vertex.eye_coordinates.normalized(), FloatVector4(0.0f, 0.0f, 0.0f, 1.0f), vector_length); auto const vertex_to_eye_point = sgi_arrow_operator(vertex.eye_coordinates.normalized(), { 0.f, 0.f, 0.f, 1.f }, vertex_to_light_length);
half_vector_normalized = vertex_to_light + vertex_to_eye_point; half_vector_normalized = vertex_to_light + vertex_to_eye_point;
} }
@ -881,9 +872,7 @@ void Device::draw_primitives(PrimitiveType primitive_type, FloatMatrix4x4 const&
specular_component = (specular * light.specular_intensity) * specular_coefficient; specular_component = (specular * light.specular_intensity) * specular_coefficient;
} }
FloatVector4 color = ambient_component; auto color = ambient_component + diffuse_component + specular_component;
color += diffuse_component;
color += specular_component;
color = color * light_attenuation_factor * spotlight_factor; color = color * light_attenuation_factor * spotlight_factor;
result_color += color; result_color += color;
} }