vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
454 stars 72 forks source link

[BUG] MaterialBuilder WithDiffuse not a valid channel #148

Closed timglewis closed 1 year ago

timglewis commented 1 year ago

Description Creating a MaterialBuilder with WithDiffuse() throws an ArgumentException

To Reproduce The following code throws the error:

Color diffuse = Color.FromArgb(-7929856);

MaterialBuilder materialBuilder = new MaterialBuilder() .WithDoubleSide(true) .WithDiffuse(new Vector4(diffuse.R, diffuse.G, diffuse.B, diffuse.A)) ;

The exception is shown here:

System.ArgumentException HResult=0x80070057 Message= Arg_ParamName_Name Source=SharpGLTF.Toolkit StackTrace: at SharpGLTF.Guard.IsTrue(Boolean target, String parameterName, String message) at SharpGLTF.Materials.MaterialBuilder.UseChannel(KnownChannel channelKey) at SharpGLTF.Materials.MaterialBuilder.WithChannelParam(KnownChannel channelKey, KnownProperty propertyName, Object parameter) at SharpGLTF.Materials.MaterialBuilder.WithDiffuse(Vector4 rgba)

The exception is thrown at this point below:

MaterialBuilder.cs: public ChannelBuilder UseChannel(KnownChannel channelKey) { Guard.IsTrue(_GetValidChannels().Contains(channelKey), nameof(channelKey)); <- Failing here

vpenades commented 1 year ago

You need to set the shader style before setting any channel information.

timglewis commented 1 year ago

Adding a metallic roughness shader didn't change the result.

        MaterialBuilder materialBuilder = new MaterialBuilder()
            .WithDoubleSide(true)
            .WithMetallicRoughnessShader()
            .WithDiffuse(new Vector4(diffuse.R, diffuse.G, diffuse.B, diffuse.A))
vpenades commented 1 year ago

Ah, yes, use BaseColor, not Diffuse

MetallicRoughness shader uses BaseColor channel and SpecularGlossiness shader (now deprecated) uses Diffuse

I know it's awkward, but it's how the glTF specification defined the channels