AcademySoftwareFoundation
Add `fractal2d` node
Mirroring the existing behavior for <fractal3d>, we add the corresponding <fractal2d> node that uses the texcoord` input as its manifold.
Add support for OSL, MSL, GLSL, MDL (note : MDL not tested).
Updated specification documents to add <fractal2d> to the specification and remove it from the proposals document.
Also remove period inputs for the noises that don't have those inputs in the specification. This is already documented in the proposals document.
Note : in the GLSL noise library code I renamed the existing mx_fractal_noise_xxx() functions to mx_fractal3d_noise_xxx() to make the differentiation cleaner with the new mx_fractal2d_noise_xxx(). But I retained wrapper functions with the old name that just call through to the new name for backwards compatibility. I was unsure how much custom customer code might be referencing those functions. We can remove the shim functions when we have another breaking API release.
MDL is broken in this state!
I'll prepare a fix and if you are okay, I'll try to push it your branch.
One question though. It seems you copied all the 3d functions with only one change:
in 2D there is an position offset of: mxp_p+float3(19, 193)
while in 3D, these is: mxp_p+float3(19, 193, 17)
do we really need this difference here? we could use the same functions if you pass the 17 as well in 2D. Since it's a new function, I don't see why not. (probably the same for the other backends of course to have matching results)
MaterialX-mdl-fractal2d-node.patch given the time zone difference and the scope of the patch, you can apply it easily yourself. If you agree with the proposal above to use the same function for 2D and 3D, let me know. I can help out as well.
I applied the patch provided by @krohmerNV. I didn't realize that we have no MDL evaluation tests in the CI - we should try and work towards fixing that.
On the topic of reusing the 3D noise calls to provide the 2D fractal node - If I'm understanding the code - the 3D version is more expensive - so I think maybe it does make sense to keep separate 2D and 3D versions. Certainly I would propose we leave that work to a follow up PR where someone can profile the changes across all the languages and noise types - and in some cases it might be a win - I'm not sure.
@ld-kerley On MDL code validation using mdlc, this has long been a goal for the MaterialX project, and I believe @krohmerNV has been steadily working towards this goal!
On the proposed change to 3D noise functions, I think Kai is referring to the TODO above, which suggests that mx_fractal_noise_float would be renamed to mx_fractal3D_noise_float for consistency with the functions that you've added here.
I think that's a great idea, if you still support the idea of this rename, and it would make this changelist more complete and self-consistent.
I only looked into the MDL implementation of the fractal3D and fractal2D functions and the only difference I saw was a +17 floating point add. What I was suggesting is to have one e.g. fractalXD_float2(float3 pos, ...) and:
fractal2D_float2 calls fractalXD_float2(pos=(texcoord.xy, 0)
fractal3D_float2 calls fractalXD_float2(pos=(hitpoint.xyz, 0)
on the MDL level I could remove one of the functions in noise.mdl when the only difference would be eliminated:
the 2D function has no +17: noise.mdl#L515
the 3D function has a +17: noise.mdl#L586
again. without this difference, we could remove the new 2D one again, and just call the general one from stdlib_1_6.mdl.
Apart from MDL. If you say 3D is more expensive, then we should keep two separate implementations. Otherwise, the pattern above could be implemented as a graph.
Saw the renaming. Thanks. This looks more consistent now. About unifying the 2D and 3D code I was asking for. It's not a blocker. If you don't want to align those two, I'm fine.
I think the implementation behind the 2D noise functions does less work than the 3D version - at least thats how I'm reading things. So it seems to make sense to leave them separate - if I'm incorrect here - or we would prefer less code at the expense of slightly slower 2D noise - I'm happy to merge them together. I don't really have a strong opinion on MDL side of things and happily defer to you.
tracked down the GLSL implementations and you are right, there is a difference. 2D is using bilinear interpolation, 3D trilinear. I also checked the MDL implementation and the PR changes are sufficient already. I wasn't aware there are 2D and 3D overloads of the perlin noise functions that serve as basis.
@ld-kerley Now that the Standard Nodes have been moved to their own document, in support of upcoming specification work at the AOUSD, this PR will need a minor adjustment to take the new location into account:
https://github.com/AcademySoftwareFoundation/MaterialX/blob/main/documents/Specification/MaterialX.StandardNodes.md
Do you have documentation for running the render tests at reference quality and how to compose the pdf for sharing? I couldn't find the documentation for these steps anywhere.
@ld-kerley Here's our documentation for the MaterialX Render Test Suite, including the full set of steps for generating output images across languages, and generating the final HTML page for comparisons. Please ask questions if you run into trouble with these instructions, and we know they can be improved!
https://github.com/AcademySoftwareFoundation/MaterialX/tree/main/source/MaterialXTest#3-shader-generation-and-render-test-suite
I had read that page - but missed any mention of "reference quality" - does the CTest run of the render tests (which I have enabled locally by default) run at the required quality level by default? If so then I believe I'm generating the images already.
Then I think the only part I'm missing are instructions on how you create the pdf in the correct format for sharing here. Are those steps documented anywhere?
Oh, good questions, @ld-kerley.
So for the reference quality setting, that's found here. You're completely correct that we don't yet document this, and we definitely should. For finding any unintended visual changes in a single commit, you could use normal-quality renders as well, as long as you have a normal-quality render from before and after your changes. I mainly focus on the high-quality renders so that I can keep a running history of our Render Test Suite output, looking for progress and/or regressions over a longer time period.
For the PDF generation, that's completely optional, and you can always just screenshot the output HTML page as needed. My own approach for generating the PDF is to load the HTML file into Chrome, and then right-click on the page to "Print" it to a PDF, but I don't believe this is necessarily the only (or even best) approach.
I ran the render tests - the test_to_html wasn't working for so just posting the before after pictures of the test here
main |
PR |
|
|---|---|---|
GLSL |
||
OSL |
Appears that this PR doesn't do any harm to the existing fractal code.