GPUOpen-Effects / TressFX

DirectX 12 and Vulkan libraries that provide convenient access to realistically rendered and simulated hair and fur
MIT License
766 stars 129 forks source link

Weird glitchy render with the preview #1

Closed ExtReMLapin closed 8 years ago

ExtReMLapin commented 8 years ago

TressFX 2.2 render : http://puu.sh/mKo82/21b2cdd9f6.jpg TressFX 3.0 render : http://puu.sh/mKo9U/11de724e15.jpg

The hair on the 3.0 is glitchy, i had to zoom so it's easier to see.

Built at the same hour, on the same computer.

jstewart-amd commented 8 years ago

Thanks for the feedback. What card and what driver?

For driver version, I'm interested in the Driver Packaging Version. For pre-Crimson drivers, it's in Catalyst Control Center, Information -> Software. It will be a long number. For example: 15.201.1151.1008-151104a-296217E

In Crimson drivers, you get to the information in a different way. But if you are running Crimson, you can just say, "I'm running Crimson."

Thanks again.

ExtReMLapin commented 8 years ago

Using a R9 290 Card

I'm running Crimson (16.1) with package :

Driver Packaging Version 15.301.1201-151223a-297971C

Thanks

jstewart-amd commented 8 years ago

Thanks again. I'll look into it.

jmackay commented 8 years ago

Yep having lots of issues with it as well. Running Crimson 16.1 hotfix with a 290 4gb card. No issues in games.

The preview of the bear here: https://raw.githubusercontent.com/GPUOpen-Effects/TressFX/master/amd_tressfx_viewer/media/Thumbnail.png

shows lighting correctly, but when trying to run it myself I see lots of issues as seen here:

https://youtu.be/-6roJowfmxI

Issues shown in video: when walking there are lots of flashs and black areas that appear. When not animating there are odd "wiggly" areas that I tried to zoom in on, but its also not clean like TressFx 2 demo.

Used Visual Studio 2015 for both, TressFx 2 looks same as OP, nice clean lines. TressFx 3 is very blocky and otherwise, glitchy looking as described and shown in the video above (video seems to be blurry until ~10 seconds in, not sure if thats just for me or youtube is still processing, but quality is normal after 10 seconds).

starquail commented 8 years ago

@jmackay I believe that the flashes of black that you are seeing are caused by the bear mesh clipping through the hair strands, which in turn is caused by the skinning animation and hair simulation being out-of-sync.

The blocky look and the strange "wiggly" effect that are apparent in your video (and in ExtReMLapin's images) look like they're being caused by some sort of driver glitch. It's hard to tell for sure, but it looks like the PPLL strand sorting algorithm is going haywire, among other things. Could be a bug in Crimson 16.1 or perhaps there's something wrong with the shader compiler's specific treatment of the Hawaii architecture, but something is definitely a bit off.

I was actually just thinking about upgrading the old Catalyst 15.7 drivers that I'm currently using with my Radeon 7950, but now I'm afraid to. =P Hopefully AMD will be able to fix things quickly, whatever the cause.

I do have a quick hack that can help with the animation synchronization issue. In the code that AMD provided, they have the hair simulation operating at a fixed frequency, which is controlled by the _target_framerate member of the _TressFXDesc structure. The mesh skinning animation updates once-per-frame.

The hack involves forcing the hair simulation to also update once-per-frame. In TressFXSimulation.cpp, find the TressFXSimulation::Simulate function and comment out the following lines:

if((m_elapsedTimeSinceLastSim < targetFrameRate) && !warp)
    {
    return S_OK;
    }

You'll also need to change

    pCSPerFrame->timeStep = targetFrameRate;

to

    pCSPerFrame->timeStep = fElapsedTime;

Please note that while the modified simulation still appears to be stable enough for short fur, I haven't tested it on longer human hair. There is a warning in the comments about the simulation being sensitive to frame rate, so your results may vary.

A better solution might be to break the hair simulation into two parts. The first part would execute every frame in order to keep the positions of the hair strands in sync with the skinned mesh. The second part would execute at the specified _target_framerate frequency and would perform the more computationally expensive physics simulation.

Even after I corrected for the simulation synchronization issue, I noticed that the fur still tended to shimmer a bit as the bear walked. This was apparently due to numerical instability in the simulation, as I was able to eliminate it by increasing the Local shape matching iterations count (located under the Simulation Menu) to 20.

Here's a quick screen capture of what the bear looks like on my system after implementing the aforementioned fix, with hair thickness set to a smaller value than the default:

image

And here's a zoomed in portion of the bear using the stock settings:

image

Notice how the fur appears much softer and less "wiggly" than it apparently does on an R9 290 running Crimson 16.1. =)

jmackay commented 8 years ago

@starquail

Interesting thanks :), I also noticed that the change hair color option did absolutely nothing, so I don't think the color is being applied (which is also why they look like long thick strands instead of nice colored hair).

with full density and minimum thickness and supposedly green hair color (applied and then re-opened for screenshot)

http://i.imgur.com/LOPyIT1.jpg

Also trying to changing those lines seems to have fixed the shimmer issue, still looks funny but I think thats mostly just the textures not applying correctly.

I did also have to cast targetFrameRate to void:

(void)targetFrameRate;

to get it to build, since it was no longer being referenced.

starquail commented 8 years ago

@jmackay

I think that the bear's fur color is being read from a texture, since nothing happens when I change the color within the viewer either.

In your screenshot, it looks like one of the problems is that your hair strands are being rendered as fully opaque, and aren't receiving any of the self-shadowing. They also don't appear to be sorted correctly, as you can see the larger (closer) strands on the bear's front leg/torso being clipped by the smaller (farther) strands on its rear legs.

Here's a close up of what I see when using similar settings (with self-shadowing disabled):

image

Leyvin commented 8 years ago

tressfx3-bug

R7 360 Crimson 16.1.1 (15.301.1801.1001-160130a-298922E) Windows 10 (Preview Build 14251.1000)

A side-by-side for comparison sake. Only built and run the viewer so far... just providing this to denote the 'out-of-the-box' is having some issues too (some similar).

jmackay commented 8 years ago

Yep, anything we can do to fix the viewer? Really hurts the "Out of the box" look. 2.2 2 2

2 2_close

3.0 3 0

3 0_close

Even the lighting on the model looks wrong. Hair coloring does work on this model, but not on the bear.

290 Crimson 16.1.1 hotfix feb 3rd

Windows 10

Visual Studio 2015 Update 1

mrgreywater commented 8 years ago

I have the same artifacts with the Crimson Drivers and my HD7970. Both Crimson 15.12 and Crimson 16.2 show these Artifacts with my HD7970. The TressFX Viewer only works correctly with the old Catalyst 15.7.1 driver.

The Artifacts are also not appearing on Intel Graphics or Nvidia, so I it's surely a driver bug. TressFX 3.0 with Intel

The bear hair simulation is out of sync with the animation and clips through the skin on all tested systems though.

sshinderman commented 8 years ago

I'm new to TressFX but noticed in the shader (TressFxRender.hlsl) that there's a couple of macros that might affect the appearance.

define SIMPLESHADING // no specular term

define SIMPLESHADOWING // no filtering except PCF

define SUPERSIMPLESHADING // constant diffuse (doesn't change with light angle)

there's also KBUFFER_SIZE which is the number of samples to depth sort.

the VS project didn't seem to have a step to notice source changes -- the line I used to generate the .inc files was:

fxc /T ps_5_0 /E PS_KBuffer_Hair /Fh PS_KBuffer_Hair.inc TressFXRender.hlsl

ExtReMLapin commented 8 years ago

By the way it's been like a month, is there any news/fix @jstewart-amd ?

jstewart-amd commented 8 years ago

Apologies for the delay. I hope to push out a fix this week.

jstewart-amd commented 8 years ago

The original issue with hair rendering reported by @ExtReMLapin is fixed in 2734dce711db2b9c8af6eb42ce417fb956288dd1

Bear animation flashing is a separate issue. I hope to have another update for that soon. Leaving this issue open until then.

jstewart-amd commented 8 years ago

My quick attempt to improve the bear animation flashing, similar to what @starquail mentioned, caused issues with longer hair. This is going to need a deeper look. We are in the midst of the annual pre-GDC scramble, so I do not know if this will be resolved until after GDC.

Again, the original issue reported by @ExtReMLapin is fixed. I'm only talking about the bear flashing issue.

So, get latest. Things are still much improved.

jstewart-amd commented 8 years ago

I suppose I should mention that if someone wants to take a shot at improving the bear flashing in a general way that does not negatively impact the simulation of longer strands, we accept pull requests. Talk to us first, though, to tell us what your plan is, and so that multiple people are not working on it simultaneously and independently.

See CONTRIBUTING.md for contribution guidelines.

Otherwise, it is next on our to-do list for TressFX (with the major caveat that progress may be stalled until after GDC).

mrgreywater commented 8 years ago

Well, if I read the code correctly, the skinning transforms are applied to the guide hairs in the simulation, and the output positions of the guide hairs is in object space. I think instead the guide hairs should be stored relative to the respective triangleFaces/skinningTransforms, so they can be rendered at the correct location even when the simulation is completely disabled. It might be that I am missing something, but It's hard to get an overview without any kind of documentation.

jstewart-amd commented 8 years ago

@mrgreywater said: "It's hard to get an overview without any kind of documentation."

I just added overview slides to amd_tressfx/doc. These slides contain information about how the skinning works (among other things).

There was already documentation for the viewer here: amd_tressfx_viewer/doc

And for the Maya exporter here: amd_tressfx_tools/MayaPlugin/doc

ExtReMLapin commented 8 years ago

@jstewart-amd is there any plan on adding engine plugins samples like this one there

mrgreywater commented 8 years ago

My proposal to fix the skinning issue: Additionally to the skinningTransform/g_Transforms matrices, compute the inverse matrices into another RWStructuredBuffer<Transforms> here: TressFXSimulation.hlsl#L1015#L1016 Apply this inverse skinning transform when finishing with the simulation, to make sure the strand positions are stored relative to the underlying triangle, not relative to the whole object: TressFXSimulation.hlsl#L377#L381 Apply the skinning transform again when rendering the Strands, using a StructuredBuffer<Transforms> : TressFXRender.hlsl#L298#L299

If the skinning transforms are orthogonal matrices (which I think they are; I don't think there is any scaling), we can simply use the transposed matrices instead of computing the inverse. But I need to confirm that first.

I don't know what the performance overhead would be, but it's the only good solution I see right now.

Update: The skinning transforms (e.g.: g_Transforms[globalStrandIndex].tfm) are in fact affine translation & rotation transformations, so it's possible to compute the inverse quickly like so:

inline row_major float4x4 InverseAffineTransformation(row_major float4x4 R) {
    //invert rotation
    row_major float4x4 inverse = transpose(R);
    inverse._m03_m13_m23 = float3(0,0,0);
    //invert translation
    inverse._m30_m31_m32 = -mul(R._m30_m31_m32_m33, inverse).xyz;
    return inverse;
}

explanation; note they are using column major Caching the inverse is still a possibility, but not strictly necessary.

jmackay commented 8 years ago

@jstewart-amd works great thanks for fixing it!

jstewart-amd commented 8 years ago

@mrgreywater, the primary author of the TressFX simulation shaders is out of the office this week, but he's back on Monday. I'll be in full GDC scramble mode, but I'll try to coordinate with him to get this looked at next week.

jstewart-amd commented 8 years ago

@jmackay

works great thanks for fixing it!

No problem. Thanks for letting me know the fix worked for you. It's always good to get independent verification.

jstewart-amd commented 8 years ago

@ExtReMLapin

is there any plan on adding engine plugins samples

Well, we definitely recognize that having example integrations for the major engines would be useful. But we are not ready at this time to say one way or the other what our plans are for that sort of thing.

mrgreywater commented 8 years ago

I fixed the bear issue as I proposed, by storing the positions (additionally) relative to the mesh surface. I still need to clean it up before I can create a PR, and it should be tested if it's faster to have the vertex positions stored twice (relative for drawing and absolute for physics), or recalculate the absolute values whenever the physics needs it (one additional matrix multiplication per access). With this fix it is possible to reduce the simulation speed or even disable it completely, without the mesh clipping through the hair. Update: See https://github.com/GPUOpen-Effects/TressFX/pull/6

jmackay commented 8 years ago

@jstewart-amd I think this issue is resolved or are there still outstanding issues? Looks like the other fix from @mrgreywater was merged so might want to close this or is something left?

jstewart-amd commented 8 years ago

There were several unrelated issues reported in this thread. The primary ones have been addressed and so, yes, I will close the issue.

Summary:

  1. The issue from the original post reported by @ExtReMLapin was a driver regression. A shader workaround was submitted in 2734dce711db2b9c8af6eb42ce417fb956288dd1, and the underlying issue has since been fixed in the driver.
  2. There are a few things going on in @jmackay's video from https://github.com/GPUOpen-Effects/TressFX/issues/1#issuecomment-175480442:

It was also mentioned that the color picker doesn't do anything for the bear. This is expected behavior, as the bear's fur color comes from the bear model texture. I created an issue to disable the color button in the UI when the color is coming from a texture, to avoid confusion: https://github.com/GPUOpen-Effects/TressFX/issues/18