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

色プロパティのシリアライズ・デシリアライズ時に色空間をコードで明示 #933

Merged
merged 8 commits into from
May 7, 2021

Conversation

Santarh
Copy link
Contributor

@Santarh Santarh commented May 7, 2021

float[]UnityEngine.Color 間の変換において色空間を明示しないと変換できないようにした。

@Santarh Santarh requested a review from ousttrue May 7, 2021 10:59
var color = src.pbrMetallicRoughness.baseColorFactor;
param.Colors.Add("_Color", (new Color(color[0], color[1], color[2], color[3])).gamma);
param.Colors.Add("_Color",
src.pbrMetallicRoughness.baseColorFactor.ToColor4(ColorSpace.Linear, ColorSpace.sRGB)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glTF の定義として baseColorFactor は Linear 空間の色である。
また、Unity の Standard Shader の _Color プロパティは sRGB 空間の色である。
その変換を明示。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glTF Import

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -38,7 +38,7 @@ static void Export_Color(Material m, TextureExporter textureManager, glTFMateria
{
if (m.HasProperty("_Color"))
{
material.pbrMetallicRoughness.baseColorFactor = m.color.linear.ToArray();
material.pbrMetallicRoughness.baseColorFactor = m.GetColor("_Color").ToFloat4(ColorSpace.sRGB, ColorSpace.Linear);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glTF Export

@@ -160,7 +161,8 @@ public static glTF_VRM_Material CreateFromMaterial(Material m, TextureExporter t
{
case ShaderPropertyType.Color:
{
var value = m.GetColor(kv.Key).ToArray();
// No color conversion. Because color property is serialized to raw float array.
var value = m.GetColor(kv.Key).ToFloat4(ColorSpace.Linear, ColorSpace.Linear);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VRM0.x Export

@@ -23,14 +24,14 @@ public override glTFMaterial ExportMaterial(Material m, TextureExporter textureE

pbrMetallicRoughness = new glTFPbrMetallicRoughness
{
baseColorFactor = def.Color.LitColor.ToArray(),
baseColorFactor = def.Color.LitColor.ToFloat4(ColorSpace.sRGB, ColorSpace.Linear),
Copy link
Contributor Author

@Santarh Santarh May 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VRM10 Export

material.SetColor(MToon.Utils.PropColor, m.pbrMetallicRoughness.baseColorFactor.ToColor4());
if (mtoon.ShadeColorFactor != null) material.SetColor(MToon.Utils.PropShadeColor, mtoon.ShadeColorFactor.ToColor3());
material.SetColor(MToon.Utils.PropColor, m.pbrMetallicRoughness.baseColorFactor
.ToColor4(UniGLTF.ColorSpace.Linear, UniGLTF.ColorSpace.sRGB)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VRM10 Import

@@ -315,7 +305,7 @@ public static void Migrate(glTF gltf, JsonNode json)
var dst = new VRMC_materials_mtoon();

// Color
gltfMaterial.pbrMetallicRoughness.baseColorFactor = mtoon.Definition.Color.LitColor.ToFloat4();
gltfMaterial.pbrMetallicRoughness.baseColorFactor = mtoon.Definition.Color.LitColor.ToFloat4(ColorSpace.sRGB, ColorSpace.Linear);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrate VRM0.x to VRM10

}

private static Color ConvertColorSpace(this Color srcColor, ColorSpace srcColorSpace, ColorSpace dstColorSpace)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 x 2 = 4 pattern

Copy link
Contributor

@ousttrue ousttrue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

よさそう

var color = src.pbrMetallicRoughness.baseColorFactor;
param.Colors.Add("_Color", (new Color(color[0], color[1], color[2], color[3])).gamma);
param.Colors.Add("_Color",
src.pbrMetallicRoughness.baseColorFactor.ToColor4(ColorSpace.Linear, ColorSpace.sRGB)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -153,7 +152,9 @@ public static bool TryCreateParam(GltfParser parser, int i, out MaterialImportPa

if (src.emissiveFactor != null && src.emissiveFactor.Length == 3)
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ousttrue ousttrue merged commit 7263562 into vrm-c:master May 7, 2021
@ousttrue ousttrue added the 1.0 label May 18, 2021
@ousttrue ousttrue added the colorspace gamma(sRGB) / Linear label Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 colorspace gamma(sRGB) / Linear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants