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

Lighting on bear is too "flashy" #19

Open
jstewart-amd opened this issue May 23, 2016 · 5 comments
Open

Lighting on bear is too "flashy" #19

jstewart-amd opened this issue May 23, 2016 · 5 comments

Comments

@jstewart-amd
Copy link
Contributor

Though this has been improved since the initial release on GitHub, the lighting on the bear still seems a little wonky. Not sure if it is in the specular or the shadows (or both), but the lighting flashes as the bear animates. The issue goes away if you disable simulation in the UI.

We are looking into it.

@mrgreywater
Copy link
Contributor

mrgreywater commented May 23, 2016

The fur flickering issue seems to be related to the shape matching constraint. The stiffness appears to increase visually with the number of shape matching iterations. Increasing to ~20 iterations stops the hair movement almost completely. I assume the physics constraint should converge to a stable value earlier, especially since the hairs are rather short.

Simple Test:

  • Decrease the Local Shape Matching Iterations to Zero, then pause the animation: The hairs are stable.
  • Increase the Local Shape Matching Iterations to One, then pause the animation: The hairs are flickering between two states, even though there is no movement in the model whatsoever!
  • Increasing the Local Shape Matching Iterations further increases the overall stability, but also changes the stiffness (which is also kind of bad from the perspective of the artist who has to come up with a stiffness value). With more than 15 iterations, the flickering issue is more or less gone.

update:
Can somebody explain how the Shape Matching works? Why is the shape angle calculated between the x-axis and the segment vector, instead of the angle between current and next hair segment?
https://github.com/GPUOpen-Effects/TressFX/blob/master/amd_tressfx/src/Shaders/TressFXSimulation.hlsl#L580#L585

@dongsoo-amd
Copy link
Contributor

dongsoo-amd commented May 23, 2016

The choice of x-axis is arbitrary but it should be consistent during asset loading and simulation. x-axis is not to compute the angle between two segment but to compute basis vectors. Since hair has bending and twisting effects as well, simple angle between two segments are not good enough to represent both effects.

You may find more details from the paper linked below.
https://amd.box.com/s/w904brn22n3e10bkh9j89f7a2trevlyi

@mrgreywater
Copy link
Contributor

mrgreywater commented May 23, 2016

But you could represent the twist and swing as quaternion between the two segments (which is what I meant with 'angle' [sorry for the confusion]).

What I would've assumed is that the quaternion of the first vertex is built from the underlying geometry triangle, the local quaternion from the second and later strand-vertices is the swing+twist quaternion of their incident edges. But reading the amd paper, it seems it's all using the x-axis as a basis vector instead, which seems weird for me, especially since it leads to problems when the x_i_plus_1_frame_i goes close to parallel to e, the x-axis.
I might have some time next weekend to check how much the other approach would change.

To simulate accurate twist it would be necessary to store more information though, such has been done in this paper: http://www.nobuyuki-umetani.com/PositionBasedElasticRod/2014_sca_PositionBasedElasticRod.pdf#page=4 (see Formula (3), (25) and (26) )

Either way, I don't think the flicker between the two states is desired, and it probably is some simulation bug.

@vlj
Copy link

vlj commented Jun 15, 2016

Is the compute tangent code correct ? I switched from tangent computation from 2.2 code to the one used in 3.0/3.1 and it looks slightly different (darker). Although it shouldn't change anything

@vlj
Copy link

vlj commented Jun 17, 2016

nvm this was a bug on my end

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

4 participants