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

Change "Depth" in spatial materials to "Height/Depth" and attenuate input values. #15794

Closed
leiget opened this issue Jan 16, 2018 · 8 comments
Closed

Comments

@leiget
Copy link

leiget commented Jan 16, 2018

I've looked for an existing issue/request.

Godot version:
Godot custom 1968cc4

Feature description:
Change "Depth" to "Height" or "Height Map" in the spatial material and shader, and attenuate the input values so the effect isn't so extreme.

The reason I suggest this is because from what I've seen the phrase "height map" is much more prevalent than "depth map". This may be because things like bricks and stone are extremely frequent in games.

Another reason being that it is confusing for a first timer to Godot depth maps that a positive value means that something is depressing into the surface. It would be more intuitive to have positive values make things pop out of a surface, and negative values be the opposite.

Also, I believe that the depth scales should for both deep parallax and "regular" depth mapping should be attenuated. Here's some code that I changed that works much better, in my opinion, as the values aren't so extreme.

Source changes I've made in "scene/resources/material.cpp":

Line 647

From:
code += "\t\tvec2 P = view_dir.xy * depth_scale;\n";

To:
code += "\t\tvec2 P = view_dir.xy * depth_scale * -0.1;\n";

Line 665

From:
code += "\t\tvec2 ofs = base_uv - view_dir.xy / view_dir.z * (depth * depth_scale);\n";
To:
code += "\t\tvec2 ofs = base_uv - view_dir.xy / view_dir.z * (depth * depth_scale * -0.05);\n";

Line 2083

From:
Line 2083: set_depth_scale(0.01);
To:
Line 2083: set_depth_scale(0.1);

And here are the before and after results:
before_edit
after_edit

These code changes are just temporary tweaks while I work on my project and are just for showing the idea. I imagine it would be nessecary to change the "depth" in code to "height" or "HeightDepth" for easier reading, but of course that's up to you guys.

@jamie-pate
Copy link
Contributor

Rather than attenuate input values, can we just be allowed to set the scale to a much lower value? Currently the editor only supports 2 significant digits (lowest value is 0.01) adding 3 more digits (0.00001) would allow this setting to be more flexible without adjusting the formula for existing materials

@leiget
Copy link
Author

leiget commented Oct 13, 2018

The main difference in my changes is that a white pixel in the depth map texture causes the surface to look like it's popping out instead of going inward, because for the longest time in bump maps and displacement maps white makes things go up and black means they are unaffected.

@jamie-pate
Copy link
Contributor

I'd think ideally that #7f7f7f would be the midpoint and darker would go down and brighter would come up, but not sure on the technical challenge. Could also have a 'midlevel' setting as well. Similar to the displace modifier in blender.

@leiget
Copy link
Author

leiget commented Oct 13, 2018

With displacement actual geometry is moved in and out. The mid point in the displace modifier is there so that the vertices don't displace into each other. With a depth, normal, or bump shader no geometry is modified so the midpoint isn't necessary and would make the shader slower.

@jamie-pate
Copy link
Contributor

jamie-pate commented Oct 13, 2018

I know this is not actual displacement. White = maxdepth or white = mindepth seems like it will be fixed by the upcoming 'invert' setting on import.

Not sure if 'height' maps are completely different from 'depth maps' in the other sense where depth means the highest value is at the polygon surface, lowest is 'behind' but height implies that the lowest value ( be it white or black in the texture) becomes the polygon surface and the highest value is 'above'

A midpoint or at least a toggle would let you decide at material creation time which one you want, if 'height' in this sense even makes sense, ever ¯_(ツ)_/¯

The other part of your fix (attenuation) seems like it would be unnecessary if we could set the scale to a very low value, like 0.0001 where the current minimum is 0.01 for many editor settings, which I think is way too high

@jamie-pate
Copy link
Contributor

For my project I ended up just inverting and applying a curve by hand since I only have like 5 depth textures
image-1

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Jan 9, 2019
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 14, 2019
@akien-mga
Copy link
Member

I think depth was renamed to height in the vulkan branch already. (CC @reduz @fracteed)
I don't know about the attenuation of the input values.

@clayjohn
Copy link
Member

Yea. Both points are fixed. The attenuation is just an issue of properly scaling the depth value (in master) and depth has been reversed and renamed height in 4.0

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

No branches or pull requests

5 participants