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

Jump to bottom

In lighting, always normalize zero to (0, 0, 1) #14231

Merged
merged 3 commits into from
Feb 28, 2021
Merged

In lighting, always normalize zero to (0, 0, 1) #14231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f63f99
GPU: Normalize 0 to 1 always in software lighting.
unknownbrackets Feb 28, 2021
3d8e444
GE Debugger: Correct powered diffuse display.
unknownbrackets Feb 28, 2021
64484a5
GPU: Normalize zero consistently in hw transform.
unknownbrackets Feb 28, 2021
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
13 changes: 5 additions & 8 deletions GPU/Common/SoftwareTransformCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <algorithm>
#include <cmath>
#include "Common/CPUDetect.h"
#include "Common/Math/math_util.h"
#include "Common/GPU/OpenGL/GLFeatures.h"

Expand Down Expand Up @@ -266,7 +267,7 @@ void SoftwareTransform::Decode(int prim, u32 vertType, const DecVtxFormat &decVt
normal = -normal;
}
Norm3ByMatrix43(worldnormal.AsArray(), normal.AsArray(), gstate.worldMatrix);
worldnormal = worldnormal.Normalized();
worldnormal = worldnormal.NormalizedOr001(cpu_info.bSSE4_1);
}
} else {
float weights[8];
Expand Down Expand Up @@ -298,7 +299,7 @@ void SoftwareTransform::Decode(int prim, u32 vertType, const DecVtxFormat &decVt
normal = -normal;
}
Norm3ByMatrix43(worldnormal.AsArray(), normal.AsArray(), gstate.worldMatrix);
worldnormal = worldnormal.Normalized();
worldnormal = worldnormal.NormalizedOr001(cpu_info.bSSE4_1);
}
}

Expand Down Expand Up @@ -358,7 +359,7 @@ void SoftwareTransform::Decode(int prim, u32 vertType, const DecVtxFormat &decVt
break;

case GE_PROJMAP_NORMALIZED_NORMAL: // Use normalized normal as source
source = normal.Normalized();
source = normal.NormalizedOr001(cpu_info.bSSE4_1);
if (!reader.hasNormal()) {
ERROR_LOG_REPORT(G3D, "Normal projection mapping without normal?");
}
Expand Down Expand Up @@ -391,11 +392,7 @@ void SoftwareTransform::Decode(int prim, u32 vertType, const DecVtxFormat &decVt
};
auto calcShadingLPos = [&](int l) {
Vec3f pos = getLPos(l);
if (pos.Length2() == 0.0f) {
return Vec3f(0.0f, 0.0f, 1.0f);
} else {
return pos.Normalized();
}
return pos.NormalizedOr001(cpu_info.bSSE4_1);
};
// Might not have lighting enabled, so don't use lighter.
Vec3f lightpos0 = calcShadingLPos(gstate.getUVLS0());
Expand Down
8 changes: 4 additions & 4 deletions GPU/Common/TransformCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <stdio.h>

#include "Common/CPUDetect.h"
#include "GPU/GPUState.h"
#include "GPU/Common/TransformCommon.h"

Expand Down Expand Up @@ -140,7 +141,7 @@ void Lighter::Light(float colorOut0[4], float colorOut1[4], const float colorIn[
case GE_LIGHTTYPE_SPOT:
case GE_LIGHTTYPE_UNKNOWN:
lightDir = Vec3Packedf(&ldir[l * 3]);
angle = Dot(toLight.Normalized(), lightDir.Normalized());
angle = Dot(toLight.NormalizedOr001(cpu_info.bSSE4_1), lightDir.NormalizedOr001(cpu_info.bSSE4_1));
if (angle >= lcutoff[l])
lightScale = clamp(1.0f / (latt[l * 3] + latt[l * 3 + 1] * distanceToLight + latt[l * 3 + 2] * distanceToLight*distanceToLight), 0.0f, 1.0f) * powf(angle, lconv[l]);
break;
Expand All @@ -155,11 +156,10 @@ void Lighter::Light(float colorOut0[4], float colorOut1[4], const float colorIn[
// Real PSP specular
Vec3f toViewer(0, 0, 1);
// Better specular
// Vec3f toViewer = (viewer - pos).Normalized();
// Vec3f toViewer = (viewer - pos).NormalizedOr001(cpu_info.bSSE4_1);

if (doSpecular) {
Vec3f halfVec = (toLight + toViewer);
halfVec.Normalize();
Vec3f halfVec = (toLight + toViewer).NormalizedOr001(cpu_info.bSSE4_1);

dot = Dot(halfVec, norm);
if (dot > 0.0f) {
Expand Down
14 changes: 10 additions & 4 deletions GPU/Common/VertexShaderGenerator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,12 @@ bool GenerateVertexShader(const VShaderID &id, char *buffer, const ShaderLanguag
WRITE(p, "}\n");
}

if (useHWTransform) {
WRITE(p, "vec3 normalizeOr001(vec3 v) {\n");
WRITE(p, " return length(v) == 0.0 ? vec3(0.0, 0.0, 1.0) : normalize(v);\n");
WRITE(p, "}\n");
}

if (ShaderLanguageIsOpenGL(compat.shaderLanguage) || compat.shaderLanguage == GLSL_VULKAN) {
WRITE(p, "void main() {\n");
} else if (compat.shaderLanguage == HLSL_D3D9 || compat.shaderLanguage == HLSL_D3D11) {
Expand Down Expand Up @@ -798,15 +804,15 @@ bool GenerateVertexShader(const VShaderID &id, char *buffer, const ShaderLanguag

WRITE(p, " vec3 worldpos = mul(vec4(tess.pos.xyz, 1.0), u_world).xyz;\n");
if (hasNormalTess) {
WRITE(p, " mediump vec3 worldnormal = normalize(mul(vec4(%stess.nrm, 0.0), u_world).xyz);\n", flipNormalTess ? "-" : "");
WRITE(p, " mediump vec3 worldnormal = normalizeOr001(mul(vec4(%stess.nrm, 0.0), u_world).xyz);\n", flipNormalTess ? "-" : "");
} else {
WRITE(p, " mediump vec3 worldnormal = vec3(0.0, 0.0, 1.0);\n");
}
} else {
// No skinning, just standard T&L.
WRITE(p, " vec3 worldpos = mul(vec4(position, 1.0), u_world).xyz;\n");
if (hasNormal)
WRITE(p, " mediump vec3 worldnormal = normalize(mul(vec4(%snormal, 0.0), u_world).xyz);\n", flipNormal ? "-" : "");
WRITE(p, " mediump vec3 worldnormal = normalizeOr001(mul(vec4(%snormal, 0.0), u_world).xyz);\n", flipNormal ? "-" : "");
else
WRITE(p, " mediump vec3 worldnormal = vec3(0.0, 0.0, 1.0);\n");
}
Expand Down Expand Up @@ -847,7 +853,7 @@ bool GenerateVertexShader(const VShaderID &id, char *buffer, const ShaderLanguag
} else {
WRITE(p, " mediump vec3 skinnednormal = mul(vec4(0.0, 0.0, %s1.0, 0.0), skinMatrix).xyz%s;\n", flipNormal ? "-" : "", factor);
}
WRITE(p, " mediump vec3 worldnormal = normalize(mul(vec4(skinnednormal, 0.0), u_world).xyz);\n");
WRITE(p, " mediump vec3 worldnormal = normalizeOr001(mul(vec4(skinnednormal, 0.0), u_world).xyz);\n");
}

WRITE(p, " vec4 viewPos = vec4(mul(vec4(worldpos, 1.0), u_view).xyz, 1.0);\n");
Expand Down Expand Up @@ -1056,7 +1062,7 @@ bool GenerateVertexShader(const VShaderID &id, char *buffer, const ShaderLanguag
break;
case GE_PROJMAP_NORMALIZED_NORMAL: // Use normalized transformed normal as source
if (hasNormal)
temp_tc = flipNormal ? "vec4(normalize(-normal), 1.0)" : "vec4(normalize(normal), 1.0)";
temp_tc = StringFromFormat("length(%snormal) == 0.0 ? vec4(0.0, 0.0, 1.0, 1.0) : vec4(normalize(%snormal), 1.0)", flipNormal ? "-" : "");
Copy link
Owner

Choose a reason for hiding this comment

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

@unknownbrackets I think the bug is here. Two %s but one parameter.

You don't really need the sign in the length check anyway...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, I don't think I intended to have it twice. Bah.

-[Unknown]

Copy link
Owner

@hrydgard hrydgard Mar 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It's kinda ridiculous that this compiles. (I mean, I know why, but ... )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes me miss D where this wouldn't. I'm sure rust has something similar.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

Indeed, this would break with Rust formatting macros.

else
temp_tc = "vec4(0.0, 0.0, 1.0, 1.0)";
break;
Expand Down
30 changes: 30 additions & 0 deletions GPU/Math3D.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,31 @@ Vec3<float> Vec3<float>::Normalized(bool useSSE4) const
const __m128 normalize = SSENormalizeMultiplier(useSSE4, vec);
return _mm_mul_ps(normalize, vec);
}

template<>
Vec3<float> Vec3<float>::NormalizedOr001(bool useSSE4) const {
const __m128 normalize = SSENormalizeMultiplier(useSSE4, vec);
const __m128 result = _mm_mul_ps(normalize, vec);
const __m128 mask = _mm_cmpunord_ps(result, vec);
const __m128 replace = _mm_and_ps(_mm_set_ps(0.0f, 1.0f, 0.0f, 0.0f), mask);
// Replace with the constant if the mask matched.
return _mm_or_ps(_mm_andnot_ps(mask, result), replace);
}
#else
template<>
Vec3<float> Vec3<float>::Normalized(bool useSSE4) const
{
return (*this) / Length();
}

template<>
Vec3<float> Vec3<float>::NormalizedOr001(bool useSSE4) const {
float len = Length();
if (len == 0.0f) {
return Vec3<float>(0.0f, 0.0f, 1.0f);
}
return *this / len;
}
#endif

template<>
Expand All @@ -154,6 +173,17 @@ float Vec3<float>::Normalize()
return len;
}

template<>
float Vec3<float>::NormalizeOr001() {
float len = Length();
if (len == 0.0f) {
z = 1.0f;
} else {
*this /= len;
}
return len;
}

template<>
Vec3Packed<float> Vec3Packed<float>::FromRGB(unsigned int rgb)
{
Expand Down
2 changes: 2 additions & 0 deletions GPU/Math3D.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,9 @@ class Vec3
Vec3 WithLength(const float l) const;
float Distance2To(Vec3 &other);
Vec3 Normalized(bool useSSE4 = false) const;
Vec3 NormalizedOr001(bool useSSE4 = false) const;
float Normalize(); // returns the previous length, which is often useful
float NormalizeOr001();

T& operator [] (int i) //allow vector[2] = 3 (vector.z=3)
{
Expand Down
14 changes: 7 additions & 7 deletions GPU/Software/Lighting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
// Official git repository and contact information can be found at
// https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.

#include "../GPUState.h"

#include "Lighting.h"
#include "Common/CPUDetect.h"
#include "GPU/GPUState.h"
#include "GPU/Software/Lighting.h"

namespace Lighting {

Expand Down Expand Up @@ -53,7 +53,7 @@ void Process(VertexData& vertex, bool hasColor) {
if (gstate.getUVGenMode() == GE_TEXMAP_ENVIRONMENT_MAP) {
Vec3<float> L = GetLightVec(gstate.lpos, light);
// In other words, L.Length2() == 0.0f means Dot({0, 0, 1}, worldnormal).
float diffuse_factor = L.Length2() == 0.0f ? vertex.worldnormal.z : Dot(L.Normalized(), vertex.worldnormal);
float diffuse_factor = Dot(L.NormalizedOr001(cpu_info.bSSE4_1), vertex.worldnormal);

if (gstate.getUVLS0() == (int)light)
vertex.texturecoords.s() = (diffuse_factor + 1.f) / 2.f;
Expand All @@ -77,7 +77,7 @@ void Process(VertexData& vertex, bool hasColor) {
L -= vertex.worldpos;
}
// TODO: Should this normalize (0, 0, 0) to (0, 0, 1)?
float d = L.Normalize();
float d = L.NormalizeOr001();

float att = 1.f;
if (!gstate.isDirectionalLight(light)) {
Expand All @@ -89,7 +89,7 @@ void Process(VertexData& vertex, bool hasColor) {
float spot = 1.f;
if (gstate.isSpotLight(light)) {
Vec3<float> dir = GetLightVec(gstate.ldir, light);
float rawSpot = dir.Length2() == 0.0f ? 0.0f : Dot(dir.Normalized(), L);
float rawSpot = Dot(dir.NormalizedOr001(cpu_info.bSSE4_1), L);
float cutoff = getFloat24(gstate.lcutoff[light]);
if (rawSpot >= cutoff) {
float conv = getFloat24(gstate.lconv[light]);
Expand Down Expand Up @@ -123,7 +123,7 @@ void Process(VertexData& vertex, bool hasColor) {
Vec3<float> lsc = Vec3<float>::FromRGB(gstate.getSpecularColor(light));
Vec3<float> msc = (materialupdate & 4) ? vcol0 : Vec3<float>::FromRGB(gstate.getMaterialSpecular());

float specular_factor = Dot(H.Normalized(), vertex.worldnormal);
float specular_factor = Dot(H.NormalizedOr001(cpu_info.bSSE4_1), vertex.worldnormal);
float k = gstate.getMaterialSpecularCoef();
specular_factor = pspLightPow(specular_factor, k);

Expand Down
3 changes: 2 additions & 1 deletion GPU/Software/TransformUnit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <cmath>
#include <algorithm>

#include "Common/CPUDetect.h"
#include "Common/Math/math_util.h"
#include "Common/MemoryUtil.h"
#include "Core/Config.h"
Expand Down Expand Up @@ -256,7 +257,7 @@ VertexData TransformUnit::ReadVertex(VertexReader& vreader)
break;

case GE_PROJMAP_NORMALIZED_NORMAL:
source = vertex.normal.Normalized();
source = vertex.normal.NormalizedOr001(cpu_info.bSSE4_1);
break;

case GE_PROJMAP_NORMAL:
Expand Down
2 changes: 1 addition & 1 deletion Windows/GEDebugger/TabState.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ void FormatStateRow(wchar_t *dest, const TabStateRow &info, u32 value, bool enab
const char *lightComputations[] = {
"diffuse",
"diffuse + spec",
"pow(diffuse) + spec",
"pow(diffuse)",
"unknown (diffuse?)",
};
const char *lightTypes[] = {
Expand Down