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

OSL built-in displace() and bump() were documented but never implemented #887

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

OSL built-in displace() and bump() were documented but never implemented #887

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
18 changes: 10 additions & 8 deletions src/doc/languagespec.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3823,26 +3823,28 @@ \section{Displacement functions}
\index{functions!displacement}

\apiitem{void {\ce displace} (float amp) \\
void {\ce displace} (vector offset) \\
void {\ce displace} (string space, float amp) \\
void {\ce displace} (vector offset)}
void {\ce displace} (string space, vector offset)}
\indexapi{displace()}

Displace the surface in the direction of the shading normal \N by
\emph{amp} units as measured in the named \emph{space} (or \commonspace
if none is specified). Alternately, the surface may be moved by a fully
general \emph{offset}, which does not need to be in the direction of the
surface normal.
\emph{amp} units, or by an arbitrary offset vector.
If an optional \emph{space} name is supplied, the amplitude or offset
are expressed in the named coordinte system (rather than the default
of \commonspace).

In either case, this function both displaces the surface and adjusts the
shading normal \N to be the new surface normal of the displaced surface
For all cases, the surface itself is displaced and the shadingnormal \N is
adjusted to be the new surface normal of the displaced surface
(properly handling both continuously smooth surfaces as well as
interpolated normals on faceted geometry, without introducing faceting
artifacts).
\apiend

\apiitem{void {\ce bump} (float amp) \\
void {\ce bump} (vector offset) \\
void {\ce bump} (string space, float amp) \\
void {\ce bump} (vector offset)}
void {\ce bump} (string space, vector offset)}
\indexapi{bump()}

Adjust the shading normal \N to be the surface normal as if the
Expand Down
2 changes: 0 additions & 2 deletions src/liboslcomp/typecheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2036,7 +2036,6 @@ static const char * builtin_func_args [] = {

"area", "fp", "!deriv", NULL,
"arraylength", "i?[]", NULL,
"bump", "xf", "xsf", "xv", "!deriv", NULL,
"calculatenormal", "vp", "!deriv", NULL,
"cellnoise", NOISE_ARGS, NULL,
"concat", "sss", /*"ss.",*/ NULL, // FIXME -- further checking
Expand All @@ -2046,7 +2045,6 @@ static const char * builtin_func_args [] = {
"Dx", "ff", "vp", "vv", "vn", "cc", "!deriv", NULL,
"Dy", "ff", "vp", "vv", "vn", "cc", "!deriv", NULL,
"Dz", "ff", "vp", "vv", "vn", "cc", "!deriv", NULL,
"displace", "xf", "xsf", "xv", "!deriv", NULL,
"environment", "fsv.", "fsvvv.","csv.", "csvvv.",
"vsv.", "vsvvv.", "!tex", "!rw", "!deriv", NULL,
"error", "xs*", "!printf", NULL,
Expand Down
29 changes: 29 additions & 0 deletions src/shaders/stdosl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,35 @@ int hash (point p, float t) BUILTIN;


// Displacement functions
void displace (vector offset) {
P += offset;
N = normalize (calculatenormal (P));
}
void displace (float amp) {
displace (amp * N);
}
void displace (string space, float amp) {
float len = length (transform (space, N));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this do the right thing if the scale is not uniform?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think. But it might depend on what you think is "right".

Copy link
Contributor

Choose a reason for hiding this comment

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

:) Yeah... I've been using this idiom forever. It makes intuitive sense, and it does give the user some form of non-uniform control over the displacement magnitude. However, lately I've switched to using the cube-root of abs(determinant(matrix(space,"common"))) as the scale factor. This change of heart is due to my experience using non-uniformly scaled coordinate systems to control displacement. With the simple length(transform(N)) method, the results are never quite what I expect or desire, and so the presumed control it allows doesn't, in my recent opinion, live up to the expectation. As a result, I've concluded that preserving a volumetric sense of relative scale is more useful in the rare instance when a user must use a non-uniformly scaled space to control displacement magnitude. Since displacement can be viewed as a volumetric operation, I now see this as making more intuitive sense. But, all that said, the simple approach works perfectly in the case of uniformly scaled coordinates, and is cheaper to compute. And since that's by far the most likely scenario, it may be the right implementation choice.

displace (amp * len * N);
}
void displace (string space, vector offset) {
displace (transform (space, "common", offset));
}

void bump (vector offset) {
N = normalize (calculatenormal (P+offset));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it discouraged to change N in a surface shader? Most closures take a normal as an argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I would prefer for bump and displace to return a normal and point respectively, and let the shader writer manage the assignment to global variables.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is that you should call one of these in the "root" shader, which is passed in the accumulated offset or amount.

If you're randomly calling it in mid-graph nodes, perhaps multiple times, you're probably going to do weird things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, the idea was, some day (or in some renderers) we might want displace() to have additional side effects. For example, maybe it modifies dPdu, dPdv? Or do other renderer-specific things? I wanted a function that does the "assignment to global variables" part, which may someday grow to have per-renderer trickiness. The "accumulate and return total" part is already trivial.

}
void bump (float amp) {
bump (amp * N);
}
void bump (string space, float amp) {
float len = length (transform (space, N));
bump (amp * len * N);
}
void bump (string space, vector offset) {
bump (transform (space, "common", offset));
}



// String functions
Expand Down