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

[1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 #1193

Merged
merged 5 commits into from
Sep 8, 2021

Conversation

ousttrue
Copy link
Contributor

@ousttrue ousttrue commented Sep 7, 2021

  • 無かったらデフォルト名にフォールバック
  • vrm0x では texture を使っていたのが、vrm10 では image を使うようになったので無名になるのかも。対処した
  • univrm のエクスポートでは基本的に image と texture の index は一致する

#1188

@ousttrue ousttrue added this to the v0.83 milestone Sep 7, 2021
@ousttrue ousttrue requested a review from Santarh September 7, 2021 06:12
@ousttrue ousttrue changed the title [1.0] thumbnail の名前が無い時に SubAssetKey を作るの失敗するのを修正 [1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 Sep 7, 2021
@ousttrue
Copy link
Contributor Author

ousttrue commented Sep 7, 2021

FixTextureNameUnique を整理しました。
texture.name と image.name が同じになる仕様を追加。 🙏

/// </summary>
private static void FixTextureNameUnique(glTF GLTF)
{
var used = new HashSet<string>();
for (var textureIdx = 0; textureIdx < GLTF.textures.Count; ++textureIdx)
{
var gltfTexture = GLTF.textures[textureIdx];
var gltfImage = GLTF.images[gltfTexture.source];
if (!string.IsNullOrEmpty(gltfImage.uri) && !gltfImage.uri.StartsWith("data:"))
if (gltfTexture.source != textureIdx)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここに来ないことを 強く 期待している

@ousttrue ousttrue changed the title [1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 [WIP][1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 Sep 7, 2021
@ousttrue
Copy link
Contributor Author

ousttrue commented Sep 7, 2021

やりなおし

@ousttrue ousttrue force-pushed the fix10/empty_thumbnail_name branch from 4d1c45f to aad1b12 Compare September 7, 2021 09:21
@ousttrue ousttrue changed the title [WIP][1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 [1.0] thumbnail の名前が無い時に SubAssetKey を作るのに失敗するのを修正 Sep 7, 2021
{
imageName = THUMBNAIL_NAME;
}
var uniqueName = GlbLowLevelParser.FixNameUnique(used, imageName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FixNameUnique で thumbnail 用の名前を作り出す !

Copy link
Contributor

Choose a reason for hiding this comment

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

Specified な実装だが、致し方なし

Copy link
Contributor

@Santarh Santarh left a comment

Choose a reason for hiding this comment

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

よさそう

{
imageName = THUMBNAIL_NAME;
}
var uniqueName = GlbLowLevelParser.FixNameUnique(used, imageName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Specified な実装だが、致し方なし

var textureIndex = kv.Value.GetInt32();
if (textureIndex == -1)
{
meta.ThumbnailImage = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

サムネがない

else
{
var gltfTexture = gltf.textures[textureIndex];
meta.ThumbnailImage = gltfTexture.source;
Copy link
Contributor

Choose a reason for hiding this comment

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

サムネがある

var index = vrm.VrmExtension.Meta.ThumbnailImage.Value;

// empty thumbnail name
vrm.Data.GLTF.images[index].name = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

image の name が null でもロードできる

@ousttrue ousttrue merged commit 7716274 into vrm-c:master Sep 8, 2021
@ousttrue ousttrue deleted the fix10/empty_thumbnail_name branch October 27, 2021 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants