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

[Outline] Clamp outline offset from user settings #39

Closed

Conversation

m2wasabi
Copy link

@m2wasabi m2wasabi commented Apr 2, 2019

#38 の件ですが、例えばこういうのどうでしょうか。
Clampされる値はUnity Editor で設定されている制限にならいました。

@Santarh
Copy link
Owner

Santarh commented Apr 2, 2019

PR のほうありがとうございます。
輪郭線幅 0 を除外してしまうというのは悩ましいところで
「マテリアルとしては輪郭線 ON にしているが、テクスチャ制御で輪郭線幅 0 にできるようにしたい」
というモチベーションが多いのです。

@Santarh
Copy link
Owner

Santarh commented Apr 2, 2019

なるほど?今思いついたのですが

  • 幅が 0.01 以上ならその値を適用する
  • 幅が 0.01 未満なら 0 にする

という実装ならアリかなと思いました。

@Santarh
Copy link
Owner

Santarh commented Apr 2, 2019

誤差が怖いので実際にはしきい値は 0.005 とかでいいかなぁ。あまりマジックナンバーは増やしたくないですが

Revert clamp
Cut off threshold outline width
@m2wasabi
Copy link
Author

m2wasabi commented Apr 2, 2019

なるほど?今思いついたのですが

  • 幅が 0.01 以上ならその値を適用する
  • 幅が 0.01 未満なら 0 にする

という実装ならアリかなと思いました。

更新しました。
実装に落とすと、こうでしょうか?

@Santarh
Copy link
Owner

Santarh commented Apr 2, 2019

ありがとうございます。ただこのコードには問題があります。

  • 幅 0.005 未満である頂点が隣り合う頂点は、幅 0.005 未満ではない

ということから、幅 0 の場合に頂点座標を 0 に飛ばしてしまうと、元の形から逸れたとても大きな三角形を描画してしまいます。

単純に outlineWidth = (outlineWidth < 0.005) ? 0 : outlineWidth となればと思います。

@Santarh
Copy link
Owner

Santarh commented Apr 2, 2019

ちなみに頂点座標を float 4 vertex = 0; としてしまうとゼロ除算が発生する ( #36 ) ので、オススメしないです。

@m2wasabi
Copy link
Author

シンプルに outlineWidth = 0 にしてしまうと、 #38 の現象に見舞われるので、Z-Fightingしないなvertexを返す必要があります。
その状態をなんとか回避したいです。

ゼロ除算しない・そこそこ隠れて見える float4(0, 0, 1, 0.0001) とかに設定してみました。
数学的正しさではなく対症療法的なアプローチですが

@Santarh
Copy link
Owner

Santarh commented May 9, 2019

PullRequest の方ありがとうございます。
せっかく出していただいて申し訳ないのですが #38 に書かせていただいたとおり別の対処法にて解決したいと思うので、こちらの方閉じさせていただきます。

@Santarh Santarh closed this May 9, 2019
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