appleseedhq / appleseed

A modern open source rendering engine for animation and visual effects
https://appleseedhq.net/
MIT License
2.2k stars 330 forks source link

Remove microfacet distribution params from OSL shaders #2688

Open dictoon opened 5 years ago

dictoon commented 5 years ago

For instance, in src/appleseed.shaders/src/appleseed/as_metal.osl we currently have this:

int in_distribution = 0
[[
    string as_maya_attribute_name = "distribution",
    string as_maya_attribute_short_name = "mdf",
    string widget = "mapper",
    string options = "Beckmann:0|GGX:1",
    string label = "Distribution",
    string page = "Specular",
    int as_maya_attribute_connectable = 0,
    int as_maya_attribute_keyable = 0,
    int as_maya_attribute_hidden = 1,
    int as_blender_input_socket = 0,
    int gafferNoduleLayoutVisible = 0,
    int divider = 1,
    int as_deprecated = 1
]],

But the value of this parameter is now ignored.

There are likely several other shaders that require updating.

Important: We need to make sure that removing these parameters don't break backward-compatibility. If that's not possible, then we should at least make it clear that they are deprecated.

luisbarrancos commented 5 years ago

@dictoon i think it's only those 3 (glass, metal, plastic), they are marked as deprecated. Removing them will break existing Maya scenes (one can add attributes, but removing them breaks scenes). I have no idea if in Max the scenario is the same. Iterating over the shader graph to find offending shaders (v1) and disconnect the attrs, remap to new (v2) mapping and rewire everything again would be one solution, but for this i would wait until some functionality is in place in order to write v2 shaders (layered closures mostly, volume closures in OSL).

est77 commented 5 years ago

Parameters can be removed from Maya nodes. You have a warning, but scenes don't break. Unknown attributes are skipped.

luisbarrancos commented 5 years ago

@est77 great, i'll remove this and add a ticket for a script that rewires shading networks. The parameter never allowed upstream connections anyway so it should be safe.

dictoon commented 5 years ago

Parameters can be removed from Maya nodes. You have a warning, but scenes don't break. Unknown attributes are skipped.

That's great! I don't think these shaders can be used in Max, at least not officially, so we should be good there. I'm not sure about Blender though. @jdent02 Will removing deprecated parameters break existing Blender scenes that rely on those shaders?

jdent02 commented 5 years ago

I don't think so. The parameter would be saved in any existing .blend but I think it just gets ignored if the shader no longer has it.

dictoon commented 5 years ago

Thanks for the info Jon. We should test this.