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

Weird glitchy render with the preview #1

Closed
ExtReMLapin opened this issue Jan 26, 2016 · 27 comments
Closed

Weird glitchy render with the preview #1

ExtReMLapin opened this issue Jan 26, 2016 · 27 comments

Comments

@ExtReMLapin
Copy link

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
Copy link
Contributor

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
Copy link
Author

Using a R9 290 Card

I'm running Crimson (16.1) with package :

Driver Packaging Version
15.301.1201-151223a-297971C

Thanks

@jstewart-amd
Copy link
Contributor

Thanks again. I'll look into it.

@jstewart-amd jstewart-amd self-assigned this Jan 26, 2016
@jmackay
Copy link

jmackay commented Jan 27, 2016

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
Copy link

@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_frame_rate member of the TressFX_Desc 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_frame_rate 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
Copy link

jmackay commented Jan 28, 2016

@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
Copy link

@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
Copy link

Leyvin commented Feb 3, 2016

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
Copy link

jmackay commented Feb 15, 2016

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
Copy link
Contributor

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
Copy link

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
Copy link
Author

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

@jstewart-amd
Copy link
Contributor

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

@jstewart-amd
Copy link
Contributor

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

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

@jstewart-amd
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

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

@mrgreywater
Copy link
Contributor

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
Copy link

jmackay commented Mar 4, 2016

@jstewart-amd works great thanks for fixing it!

@jstewart-amd
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

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 #6

@jmackay
Copy link

jmackay commented May 19, 2016

@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
Copy link
Contributor

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 2734dce, and the underlying issue has since been fixed in the driver.
  2. There are a few things going on in @jmackay's video from Weird glitchy render with the preview #1 (comment):

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: #18

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

7 participants