Addition of Ashikhmin-Shirley anisotropic BSDF to Cycles
ClosedPublic
Actions

Authored by Karsten Schwenk (macnihilist) on May 25 2014, 11:04 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
Thomas Dinges (dingto)

Commits

rBS8ce1090d4e81: Cycles: Ashikhmin-Shirley anisotropic BSDF
rC774b633ad404: Cycles: Ashikhmin-Shirley anisotropic BSDF
rB8ce1090d4e81: Cycles: Ashikhmin-Shirley anisotropic BSDF

Summary

Ashikhmin-Shirley anisotropic BSDF was added as closure
WardBSDFNode renamed to AnisotropicBSDFNode; has now two distributions (Ward and Ashikhmin-Shirley)
Fresnel factor was left out (as in all other microfacet BSDFs)
IMPORTANT: Was not able to test OSL integration, because my build crashes with any OSL rendering without usable callstack (maybe something is not correctly configured for the linker). So I'm afraid I have to put that burden on somebody else's shoulders (Thomas Dinges?)
OpenCL not tested for similar reasons

Update:

OSL fixes (could still not test it, though)
app/cycles_xml.cpp updated (for backwards compatibility one could create a AnisoNode with Ward distribution for a "ward_bsdf" string, but I don't know what the policy is there, so I didn't do it)

Diff Detail

Event Timeline

Hey Karsten,
thanks for the patch, nice addition! :)

For OSL, 2 things are wrong / missing:
*node_anisotropic_bsdf.osl: The first function call should be ashikhmin_shirley(), not ward().
*stdosl.h: You need to add the closure here:

closure color ashikhmin_shirley(normal N, vector T,float ax, float ay) BUILTIN;

Apart from that the patch works for me, and render in OSL and SVM then. Here a comparison:

I have not reviewed the code extensively yet though.

Nice work! Good to see that this BSDF suffers much less from the edge darkening problem than Ward.

Should I submit a new version with dingto's fixes, or is that not necessary?

Also: I noticed that I forgot to update app/cycles_xml.cpp (still uses WardBsdfNode)

It would be nice to submit an update with the fixes. If you did this via the Web GUI here, you can click on Submit Code again on the main page, and then attach the .diff to this revision (instead of submitting a new).

Thanks!

Karsten Schwenk (macnihilist) updated this revision to Unknown Object (????).May 27 2014, 5:58 PM

Some minor comments. I intend to merge this after git is open again along with D572: Improved importance sampling for Beckmann and GGX.

intern/cycles/kernel/closure/bsdf_ashikhmin_shirley.h
1	This file needs a license header (you can copy it from another file).
21	Code style in other files is to write `1.0f` rather than `1.f`, can you change this?
intern/cycles/util/util_math.h
1110–1113	Any particular reason this was changed?

intern/cycles/util/util_math.h:1110-1113 Any particular reason this was changed?

This was included 'for your consideration' (it's slightly shorter than the original, but I don't think there is anything wrong with the original). It is excluded now. I changed it during debugging because I suspected a precision issue, but the problem was something else. Probably should have submitted it separately (if at all), sorry.