FC2ブログ

スポンサーサイト

上記の広告は1ヶ月以上更新のないブログに表示されています。
新しい記事を書く事で広告が消せます。

リファクタリング実例 OpenSceneGraph TangentSpaceGenerator

普段コードのリファクタリングをすることが良くあって、いつか作業の例を紹介したいなと思っていたのですが、
中々業務用のコードを公開するわけには行きませんので機会が無かったのですが、最近オープンソースの
コードリーディングをすることがあって、そのときに行ったリファクタリングの例を紹介したいと思います。


対象のコード

https://github.com/openscenegraph/OpenSceneGraph/blob/OpenSceneGraph-3.6.0/src/osgUtil/TangentSpaceGenerator.cpp

e0eb4fb on 21 Nov 2017
のリビジョンのコードの一部をリファクタリングしていきます。

これは、OpenSceneGraphという3DCG用のライブラリのコードの一部で、3D形状の接方向を推定するコードになっています。

コードの詳細は省略しますが、概要としては、まずこのモジュールに3D形状の一部を構成刷る三角形が与えられます。
で、その三角形の各頂点に対して接方向、及び従法線の方向を推定する、という内容になっています。

各変数の詳細説明は省略しますが、大体下記の命名ルールとなっているようです

P : position
N : normal(法線)
T : tangent(接線)
B : Binormal(従法線)


問題箇所抜粋

完全なコピペではないのですが、初期状態で次のような状態になっています。




やけに面倒なコードになっていますので、順を追ってコードをスリムにしていきます。



手順

まずはじめに下記の変換をやっていきます。

  • 冗長なコードを変数に抽出
  • 不要な代入をインライン化
  • コードを関連する処理ごとにグルーピング


これらの編集を加えたコードが次になります。




この段階ではコード量は増えてしまっていますが、以降の手順でどんどん圧縮されていきます。

続いて、次のステップでは単純なコードの調整だけではなく、理論を若干考慮した変換を行っていきます。

  • vcx.x, vcy.x, vcz.xが同値なので、参照する変数をまとめる+if文も1つにまとめる。
  • normalizeは不要なので削除する。

それぞれ、根拠はベクトルの計算を考えればわかります。

で、それらの変換を行ったコードがこちら。




このような状態にすると明らかになるのですが、実は単なるベクトルで計算すれば良いところが、成分単位で記述されているところが多く見られますので、そのあたりの不要なコードを省略してやります。




コードがこの状態になると、実はT123、B123が不要であることがわかります。
それぞれ、一回0で初期化される→値を1回加算される→1回参照される
というだけの流れになるので、不要な加算、代入を省略することができます。

また、if文はガード節に変換してやります。




ここまで来ると、実は外積がほとんど働いていないことがわかりますので、すべてインライン展開してやります。




で、次が最後。

コードがこの状態になると、またここでも本来ベクトル単位での演算で良いところを、成分単位で計算してしまっていますので、
省略してやります。




で以上になります。

最初のコードと比較すると、元々57行あったのが、17行と3分の1以下まで圧縮出来ることがわかります。



普段コードのリファクタリングをすることが良くあって、いつか作業の例を紹介したいなと思っていたのですが、
中々業務用のコードを公開するわけには行きませんので機会が無かったのですが、最近オープンソースの
コードリーディングをすることがあって、そのときに行ったリファクタリングの例を紹介したいと思います。


対象のコード

https://github.com/openscenegraph/OpenSceneGraph/blob/OpenSceneGraph-3.6.0/src/osgUtil/TangentSpaceGenerator.cpp

e0eb4fb on 21 Nov 2017
のリビジョンのコードの一部をリファクタリングしていきます。

これは、OpenSceneGraphという3DCG用のライブラリのコードの一部で、3D形状の接方向を推定するコードになっています。

コードの詳細は省略しますが、概要としては、まずこのモジュールに3D形状の一部を構成刷る三角形が与えられます。
で、その三角形の各頂点に対して接方向、及び従法線の方向を推定する、という内容になっています。

各変数の詳細説明は省略しますが、大体下記の命名ルールとなっているようです

P : position
N : normal(法線)
T : tangent(接線)
B : Binormal(従法線)
uv : テクスチャ座標


問題箇所抜粋

完全なコピペではないのですが、初期状態で次のような状態になっています。




やけに面倒なコードになっていますので、順を追ってコードをスリムにしていきます。

まずはじめに下記の変換をやっていきます。

  • 冗長なコードを変数に抽出
  • 不要な代入をインライン化
  • コードを関連する処理ごとにグルーピング


これらの編集を加えたコードが次になります。




この段階ではコード量は増えてしまっていますが、以降の手順でどんどん圧縮されていきます。

続いて、次のステップでは単純なコードの調整だけではなく、理論を若干考慮した変換を行っていきます。

  • vcx.x, vcy.x, vcz.xが同値なので、参照する変数をまとめる+if文も1つにまとめる。
  • normalizeは不要なので削除する。

それぞれ、根拠はベクトルの計算を考えればわかります。

で、それらの変換を行ったコードがこちら。




このような状態にすると明らかになるのですが、実は単なるベクトルで計算すれば良いところが、成分単位で記述されているところが多く見られますので、そのあたりの不要なコードを省略してやります。




コードがこの状態になると、実はT123、B123が不要であることがわかります。
それぞれ、一回0で初期化される→値を1回加算される→1回参照される
というだけの流れになるので、不要な加算、代入を省略することができます。

また、if文はガード節に変換してやります。




ここまで来ると、実は外積がほとんど働いていないことがわかりますので、すべてインライン展開してやります。




で、次が最後。

コードがこの状態になると、またここでも本来ベクトル単位での演算で良いところを、成分単位で計算してしまっていますので、
省略してやります。




で以上になります。

最初のコードと比較すると、元々57行あったのが、17行と3分の1以下まで圧縮出来ることがわかります。



スポンサーサイト

コメントの投稿

非公開コメント

カレンダー
09 | 2018/10 | 11
- 1 2 3 4 5 6
7 8 9 10 11 12 13
14 15 16 17 18 19 20
21 22 23 24 25 26 27
28 29 30 31 - - -
最新記事
カテゴリ
Qt (21)
SDL (2)
MFC (2)
検索フォーム
月別アーカイブ
最新コメント
最新トラックバック
RSSリンクの表示
リンク
リンク(管理用)
FC2カウンター
上記広告は1ヶ月以上更新のないブログに表示されています。新しい記事を書くことで広告を消せます。