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

GetBytesWithMime を VRMShaders に移動 #863

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

ousttrue
Copy link
Contributor

  • UniGLTF.GltfTextureExporter.GetBytesWithMime を VRMShaders.AssetTextureUtil.GetTextureBytesWithMime に移動
  • テスト NotReadable, Compressed を VRMShaders に移動
  • テスト用リソース Resources/4x4.png と 4x4compressed.DDS を VRMShaders に移動 Build failed for iOS in v0.70.0 & 0.71.0 #857

#758

@ousttrue ousttrue requested a review from Santarh April 12, 2021 09:10
@ousttrue ousttrue added this to the v0.72 milestone Apr 12, 2021
* UniGLTF.GltfTextureExporter.GetBytesWithMime を VRMShaders.AssetTextureUtil.GetTextureBytesWithMime に移動
* テスト NotReadable, Compressed を VRMShaders に移動
* テスト用リソース Resources/4x4.png と 4x4compressed.DDS を VRMShaders に移動
@ousttrue ousttrue force-pushed the fix/do_not_use_resources branch from 1b17158 to e35bc5f Compare April 12, 2021 09:17
/// <returns></returns>
public static (byte[] bytes, string mine) GetBytesWithMime(Texture2D texture)
{
#if UNITY_EDITOR
Copy link
Contributor

Choose a reason for hiding this comment

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

これもともと Runtime のほうにあったんですね

/// <param name="bytes"></param>
/// <param name="texture"></param>
/// <returns></returns>
public static bool TryGetBytesWithMime(Texture2D texture, out byte[] bytes, out string mime)
Copy link
Contributor

Choose a reason for hiding this comment

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

Editor 以下に mv できて……ヨシ!

Copy link
Contributor

Choose a reason for hiding this comment

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

これ以上 VrmShaders に実装を委譲するような機能が増えるなら
UniGLTF/Runtime/UniGLTF/IO/TextureIO に interface を定義してそれを実装するようにしたほうがいいかもしれない。
引数増え過ぎちゃうので。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ベースクラスに対して2方向の拡張があって、

  • VRM, VCI など Extensions としての拡張 => 継承とFactory差し替え
  • EditorのAsset操作のような拡張 => ExternalObject、Factory差し替え
    • 以前は、 #if UNITY_EDITOR だったが整理中

だいぶ集まってきたのでそろそろ interface 等の整理ができるかもしれない。

@ousttrue ousttrue merged commit 6977424 into vrm-c:master Apr 12, 2021
@ousttrue ousttrue deleted the fix/do_not_use_resources branch May 17, 2021 05:21
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