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.

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

Add normal maps #5643

Closed

Conversation

notquitehadouken
Copy link
Contributor

@notquitehadouken notquitehadouken commented Jan 30, 2025

Add normal maps to Clyde

Basically done, just waiting on content PR's spiriting work to conclude, and if #5602 is merged before that, then managing the (probably not too bad) merge conflicts

MyServer.-.Space.Station.14.2025-02-03.11-00-40.online-video-cutter.com.mp4

image

image

image

Defaults to off (toggle with the togglenormals command) for performance reasons (reasons being "if it turns out this has way worse performance on other computers or something then i don't want it to make people's ram boil alive")

Suggested to turn off light blur and to set resolution to 1

@notquitehadouken notquitehadouken changed the title Add normals (for real this time) Add normal maps Jan 30, 2025
@notquitehadouken
Copy link
Contributor Author

notquitehadouken commented Feb 6, 2025

pull the branch and run it to experience the wonders

@notquitehadouken notquitehadouken marked this pull request as draft February 6, 2025 19:58
@notquitehadouken notquitehadouken marked this pull request as ready for review February 10, 2025 14:02
@notquitehadouken notquitehadouken marked this pull request as draft February 10, 2025 14:02
@notquitehadouken
Copy link
Contributor Author

It's back

0.441 is back

image

@notquitehadouken notquitehadouken marked this pull request as ready for review February 20, 2025 00:29
Comment on lines +69 to +89

if (useNormals == 1)
{
highp vec2 lightDir = rotateVector(normalize(diff), globalRotation);
highp vec2 newUV = rotateVector(UV - vec2(0.5, 0.5) + rotateVector((lightCenter - eyeCenter) / vec2(2, -2) / lightRange, maskRotation), -globalRotation);
newUV *= (projectionMatrix * vec3(1.0, 1.0, 0.0)).xy * vec2(1.0, -1.0) / eyeZoom * lightRange;
newUV += vec2(0.5, 0.5);

highp vec4 origSample = texture2D(normalMap, newUV);
if (origSample.xyzw == vec4(0.0, 0.0, 0.0, 1.0))
discard;

// origSample is on the range 0 - 1 in all components,
// but every color component has been raised to SOMETHING by some unknown mechanism
// so we first raise it to the 0.452nd power to reset it
highp vec3 normalSample = pow(origSample.xyz, vec3(COLOR_POWER)) * vec3(2.0, 2.0, 1.0) - vec3(1.0, 1.0, 0.0);
run = dot(normalize(vec3(lightDir, lightHeight)), normalSample * vec3(-1.0, -1.0, 1.0));
if (run <= 0.0)
discard;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't if statements expensive as shit in shaders.

Would it be better to just draw the normals to a separate texture and use that or somethin

Comment on lines +32 to +39
private void _drawGrids(Viewport viewport, Box2 worldAABB, Box2Rotated worldBounds, IEye eye, bool normal = false)
{
if (normal && (!_lightManager.Enabled
|| !_lightManager.DrawLighting
|| !_cfg.GetCVar(CVars.LightNormals)
|| !_resourceCache.GetNormalsEnabled()))
return;

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really sure why drawing is dropped entirely if lighting isn't enabled?

Comment on lines +1 to +11
uniform highp float rotation;
const highp float COLOR_POWER = 0.452;
const highp float COLOR_POWERNT = 2.212;

void fragment()
{
highp vec4 Col = zTexture(UV);
Col = vec4(pow(Col.xy, vec2(COLOR_POWER)) - vec2(0.5, 0.5), Col.zw);
Col = vec4(Col.xy * cos(rotation) + vec2(-1.0, 1.0) * Col.yx * sin(rotation), Col.zw);
Col = vec4(pow(Col.xy + vec2(0.5, 0.5), vec2(COLOR_POWERNT)), Col.zw);
COLOR = Col;
Copy link
Contributor

Choose a reason for hiding this comment

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

PLEASE comment shaders, especially when using magic numbers. This also has 0 flexibility for content.

Comment on lines 36 to +41

/// <summary>
/// The width of the texture we came from, used for offsetting to get normals
/// </summary>
public int RegionWidth { get; }

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think knowing other texture areas should be something callers should worry about . AtlasTexture is just an abstraction to handle GPU memory in a more sane way.

Comment on lines 15 to +16
{
bool GetNormalsEnabled();
Copy link
Contributor

Choose a reason for hiding this comment

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

ResourceCache should not be caring about rendering, at the very least it should be on clyde.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ngl i wrote that because "It works lmaoroflxd" and intended to find a better solution later. i never returned to it

Comment on lines +231 to +240
{
var CPixel = normalImage[nX, nY].ToVector4();
var NormVector = new Vector3(CPixel.X, CPixel.Y, CPixel.Z);
NormVector = NormVector * new Vector3(2f, 2f, 1f) - new Vector3(1f, 1f, 0f);
NormVector.Normalize();
NormVector = (NormVector + new Vector3(1f, 1f, 0f)) * new Vector3(0.5f, 0.5f, 1f);
var NewColor = new Rgba32(NormVector.X, NormVector.Y, NormVector.Z, texture[nX, nY].A);
normalImage[nX, nY] = NewColor;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Please document code with magic numbers.

Comment on lines 197 to +199
var texPath = data.Path / (stateObject.StateId + ".png");
var normalPath = data.Path / (stateObject.NormalId + ".png");
var bumpPath = data.Path / (stateObject.BumpId + ".png");
Copy link
Contributor

Choose a reason for hiding this comment

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

RSIs shouldn't have knowledge of normalmaps or bumpmaps in the same way they do not have knowledge of displacement maps.

Comment on lines +63 to +88
{
int Width = bumpmap.Width;
int Height = bumpmap.Height;
normal = new Image<Rgba32>(Width, Height);
int x, y;
for (x = 0; x < Width; x++)
for (y = 0; y < Height; y++)
{
int XFactor = x / blocksize.X;
int YFactor = y / blocksize.Y;

int XL = x - 1;
int YL = y - 1;
int XR = x + 1;
int YR = y + 1;

float Left = 0f, Right = 0f, Up = 0f, Down = 0f;

if (XL - XFactor * blocksize.X >= 0)
Left = bumpmap[XL, y].R / 255f;
if (XR - XFactor * blocksize.X < blocksize.X)
Right = bumpmap[XR, y].R / 255f;
if (YL - YFactor * blocksize.Y >= 0)
Up = bumpmap[x, YL].R / 255f;
if (YR - YFactor * blocksize.Y < blocksize.Y)
Down = bumpmap[x, YR].R / 255f;
Copy link
Contributor

Choose a reason for hiding this comment

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

As elsewhere please document.

Comment on lines +124 to +141

if (XL - XFactor * blocksize.X >= 0)
Left = bumpmap[XL, y].A;
if (XR - XFactor * blocksize.X < blocksize.X)
Right = bumpmap[XR, y].A;
if (YL - YFactor * blocksize.Y >= 0)
Up = bumpmap[x, YL].A;
if (YR - YFactor * blocksize.Y < blocksize.Y)
Down = bumpmap[x, YR].A;

if (Left == 0 || Right == 0 || Up == 0 || Down == 0)
{
bumpmap[x, y] = new Rgba32(0.5f, 0.5f, 0.5f, original[x, y].A);
}
else
{
bumpmap[x, y] = new Rgba32(1f, 1f, 1f, original[x, y].A);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As elsewhere please document.

Copy link
Contributor

@metalgearsloth metalgearsloth left a comment

Choose a reason for hiding this comment

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

Next time I'll do a proper review.

At the moment this is heavily coupled to engine and content has no control over it. Also if we need to make changes it now affects everything else.

  1. Layers should not have an entirely separate shader for normals vs non-normals when only one is used.
  2. Baking normal into every method signature is going to make it unmaintainable. E.g. extracttexture should not be caring about normals, texture drawing shouldn't care, dependency loading shouldn't care, etc.

We managed to get displacement maps without requiring it to be tightly coupled to the engine and we won't need it for normals.

@notquitehadouken notquitehadouken marked this pull request as draft February 21, 2025 03:57
@notquitehadouken
Copy link
Contributor Author

i will create New branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants