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] TextureTransformBind の Scale/Offset 変換 #1234

Merged
merged 5 commits into from
Sep 21, 2021

Conversation

ousttrue
Copy link
Contributor

  • UniGLTF/Samples フォルダは実験シーンなので詳しく見なくて OK
  • TextureTransformBind も入出力で変換必要
  • TextureTransform.VerticalFlipScaleOffset 関数にくくり出した

ついでに実験してみた。

  • 左がUnity左下原点
  • 右がglTF想定の Y -1 スケールで左上原点

Santarh
Santarh previously approved these changes Sep 21, 2021
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.

変換コードの一元化、LTGTM

/// <returns></returns>
public static (Vector2, Vector2) VerticalFlipScaleOffset(Vector2 s, Vector2 o)
{
return (new Vector2(s.x, s.y), new Vector2(o.x, 1.0f - o.y - s.y));
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

/// <param name="s"></param>
/// <param name="o"></param>
/// <returns></returns>
public static (Vector2, Vector2) VerticalFlipScaleOffset(Vector2 s, Vector2 o)
Copy link
Contributor

Choose a reason for hiding this comment

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

(どちらでもよい)タプルに名前を付けておいた方が、メンテ性はあるかも。

public static (Vector2 scale, Vector2 offset) VerticalFlipScaleOffset(Vector2 scale, Vector2 offset)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

たしかに。わりと OffsetScale と言っていることがある。

Copy link
Contributor

@Santarh Santarh Sep 21, 2021

Choose a reason for hiding this comment

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

計算を適用する順序としては uv * scale + offset なので ScaleOffset の方が間違いが少ないという気持ちはある
微妙なところ

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.

LGTM

@ousttrue ousttrue merged commit 024dadf into vrm-c:master Sep 21, 2021
@ousttrue ousttrue deleted the fix10/convert_TextureTransform 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