地下埋設物を対象とした平面直角座標系への対応 #640
Conversation
Walkthroughこのプルリクエストでは、CityGMLデータの座標参照系（CRS）の扱いを改善するために、複数のメソッドのシグネチャが変更され、EPSGコードをオプションとして受け取るようになりました。また、ファイル名からEPSGコードを抽出する新しいユーティリティ関数が追加され、座標変換のロジックがリファクタリングされました。これにより、地下埋設物データの処理が柔軟かつ効率的に行えるようになります。 Changes
Assessment against linked issues
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
nusamai-citygml/src/parser.rs (1)
323-325:collect_geometriesメソッドの使用に関する問題
collect_geometriesメソッドの新しいシグネチャにはOption<EpsgCode>パラメータが必要ですが、すべての使用箇所がこの変更を反映していません。特に、以下のファイルではパラメータなしで呼び出されています。
nusamai-plateau/examples/parse_and_compress.rsnusamai-plateau/tests/common/mod.rsこれにより、コンパイルエラーやランタイムエラーが発生する可能性があります。これらの呼び出しを更新して、新しいメソッドシグネチャに一致させる必要があります。
Analysis chain
EPSGコードのオプションの追加について
collect_geometriesメソッドにepsg_optパラメータを追加する変更は、座標参照系を柔軟に扱うための良い改善です。ただし、この変更が他のシステムやプロセスに予期しない影響を与えないか確認する必要があります。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new epsg_opt parameter and its impact on the system. # Test: Search for the usage of `collect_geometries` method. Expect: All usages should now include the epsg_opt parameter. rg --type rust -A 5 $'collect_geometries'Length of output: 1888
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- nusamai-citygml/src/geometry.rs (2 hunks)
- nusamai-citygml/src/parser.rs (2 hunks)
- nusamai/src/source/citygml.rs (5 hunks)
- nusamai/src/transformer/transform/projection.rs (2 hunks)
Additional comments not posted (7)
nusamai-citygml/src/geometry.rs (1)
Line range hint
125-140: メソッドのシグネチャとロジックの変更を承認します。
into_geometriesメソッドにオプショナルなEPSGコードパラメータを追加する変更は、柔軟性を向上させるための適切なアプローチです。デフォルト値としてEPSG_JGD2011_GEOGRAPHIC_3Dを使用することは、プロジェクトのコンテキストに合っています。以下のスクリプトを実行して、オプショナルパラメータの扱いが正しいかどうかを確認してください：
Verification successful
オプショナルEPSGコードパラメータの扱いは正しいです。
collect_geometriesメソッドでのepsg_optの扱いは、into_geometriesに直接渡されるため、正しく実装されています。中間のロジックがないため、意図した設計と一致しています。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of the optional EPSG code parameter in `into_geometries`. # Test: Search for the method usage. Expect: Correct handling of the optional parameter. rg --type rust -A 5 $'into_geometries'Length of output: 863
Script:
#!/bin/bash # Description: Gather more context around the usage of `into_geometries` in `parser.rs`. # Search for the definition and usage of `epsg_opt` in `parser.rs` to understand how it is set and passed. rg --type rust -A 10 'epsg_opt' nusamai-citygml/src/parser.rsLength of output: 521
nusamai/src/source/citygml.rs (3)
89-97: ファイル名からEPSGコードを抽出する機能の追加この関数はファイル名からEPSGコードを抽出し、適切にパースするためのものです。実装はシンプルで効率的ですが、エラーハンドリングがもう少し詳細に行われるとより良いでしょう。例えば、どの部分でパースに失敗したかをログに記録するなどです。
105-105: トップレベルディスパッチャー関数のシグネチャ変更この関数に新しいパラメータ
filenameが追加されました。これにより、ファイル名に基づいて動的にEPSGコードを処理することが可能になります。すべての関数呼び出しでこの新しいパラメータが適切に渡されているかを確認することをお勧めします。
237-240: 新しいテストケースの追加
query_crs_from_filename関数の機能を確認するためのテストケースが追加されました。このテストは基本的な機能をチェックしていますが、さまざまなシナリオに対応するために、より多くのテストケースを追加することをお勧めします。nusamai/src/transformer/transform/projection.rs (3)
80-168: JGD2011からの変換ロジックの改善
transform_from_jgd2011メソッドは、JGD2011地理システムに関連する変換を処理します。このメソッドにより、コードのモジュール性が向上し、読みやすさが改善されています。ただし、サポートされていないEPSGコードに対するエラーハンドリングを追加することをお勧めします。
170-217: WGS84からの変換ロジックの改善
transform_from_wgs84メソッドは、WGS84システムからの変換を処理します。このメソッドにより、コードの構造が改善され、保守が容易になります。ただし、unimplemented!を使用している部分については、必要な変換の実装を進めることをお勧めします。
27-54: メイン変換関数のリファクタリングメインの変換関数は、特定のロジックを新しいメソッドに委譲することで簡素化されています。これにより、コードの読みやすさと保守性が向上しています。すべての変換が正しく委譲されていることを確認するために、徹底的なテストを行うことをお勧めします。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- nusamai/src/source/citygml.rs (5 hunks)
- nusamai/src/transformer/transform/projection.rs (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- nusamai/src/source/citygml.rs
- nusamai/src/transformer/transform/projection.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nusamai/src/transformer/transform/projection.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- nusamai/src/transformer/transform/projection.rs
TadaTeruki
force-pushed
the
fix/unf-coord-transformation
branch
from
f861c9b
to
3a66f03
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nusamai/src/transformer/transform/projection.rs (2 hunks)
Additional comments not posted (3)
nusamai/src/transformer/transform/projection.rs (3)
27-43: リファクタリングにより、コードの可読性と保守性が向上しました。
transformメソッドのリファクタリングにより、入力座標系に基づいて実際の変換ロジックを別のメソッドに委譲することで、コードの構造が改善されました。これにより、matchステートメントの複雑さが軽減され、可読性と保守性が向上しています。また、平面直角座標系の場合に入力EPSGコードを
transform_from_jgd2011メソッドに渡すことで、そのメソッド内で適切に処理できるようになりました。
69-74:rectangular_to_lnglat関数の追加は、コードの再利用性と保守性を向上させます。
rectangular_to_lnglat関数は、平面直角座標系から経度・緯度への変換ロジックをカプセル化しています。入力EPSGコードをパラメータとして受け取ることで、異なる平面直角座標系に対応できます。また、
JPRZone構造体とその関連する投影法を利用して変換を行うことで、コードの再利用性と保守性が促進されます。
76-184:transform_from_jgd2011メソッドの実装により、様々な座標系からの変換処理が統合されました。
transform_from_jgd2011メソッドは、JGD2011を基準とする様々な座標系から出力座標系への変換ロジックを統合しています。出力EPSGコードに基づいてmatchステートメントを使用することで、各ケースに適した変換を適用しています。このメソッドは、地理座標系、高さを持つ平面直角座標系、高さを持たない平面直角座標系を個別に処理し、それぞれに適した変換が行われるようになっています。また、必要に応じて
rectangular_to_lnglat関数を使用して、平面直角座標系から経度・緯度への変換を行っています。これにより、様々な座標系からの変換処理が適切に行われ、コードの可読性と保守性が向上しています。
| fn transform_from_wgs84(&mut self, _entity: &Entity, _rectangular: Option<EpsgCode>) { | ||
| match self.output_epsg { | ||
| EPSG_WGS84_GEOGRAPHIC_3D => { | ||
| // Do nothing | ||
| } | ||
| EPSG_JGD2011_JPRECT_I_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_II_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_III_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_IV_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_V_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_VI_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_VII_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_VIII_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_IX_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_X_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_XI_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_XII_JGD2011_HEIGHT | ||
| | EPSG_JGD2011_JPRECT_XIII_JGD2011_HEIGHT => { | ||
| // TODO: implement | ||
| unimplemented!("WGS84 to EPSG:{} not supported yet", self.output_epsg); | ||
| } | ||
| EPSG_JGD2011_JPRECT_I | ||
| | EPSG_JGD2011_JPRECT_II | ||
| | EPSG_JGD2011_JPRECT_III | ||
| | EPSG_JGD2011_JPRECT_IV | ||
| | EPSG_JGD2011_JPRECT_V | ||
| | EPSG_JGD2011_JPRECT_VI | ||
| | EPSG_JGD2011_JPRECT_VII | ||
| | EPSG_JGD2011_JPRECT_VIII | ||
| | EPSG_JGD2011_JPRECT_IX | ||
| | EPSG_JGD2011_JPRECT_X | ||
| | EPSG_JGD2011_JPRECT_XI | ||
| | EPSG_JGD2011_JPRECT_XII | ||
| | EPSG_JGD2011_JPRECT_XIII | ||
| | EPSG_JGD2011_JPRECT_XIV | ||
| | EPSG_JGD2011_JPRECT_XV | ||
| | EPSG_JGD2011_JPRECT_XVI | ||
| | EPSG_JGD2011_JPRECT_XVII | ||
| | EPSG_JGD2011_JPRECT_XVIII | ||
| | EPSG_JGD2011_JPRECT_XIX => { | ||
| // TODO: implement | ||
| unimplemented!("WGS84 to EPSG:{} not supported yet", self.output_epsg); | ||
| } | ||
| _ => { | ||
| panic!("Unsupported output CRS: {}", self.output_epsg); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
transform_from_wgs84メソッドの実装が未完了です。今後の実装が必要です。
transform_from_wgs84メソッドは、WGS84を入力座標系としてサポートするための基礎を設定していますが、実際の変換処理の実装はまだ完了していません。unimplemented!マクロの使用により、さらなる開発が必要な箇所が明確に示されています。
また、サポートされていない出力座標系を処理するためにパニックステートメントが含まれており、エラーを明示的にキャッチして処理するのに役立ちます。
必要な変換処理の実装を完了させるために、追加のサポートが必要でしょうか？もしよければ、実装の完了をお手伝いさせていただきます。GitHubのイシューを作成して、このタスクを追跡することもできます。
TadaTeruki
force-pushed
the
fix/unf-coord-transformation
branch
3 times, most recently
from
7e001c5
to
aa0c909
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
📢 Thoughts on this report? Let us know! |
|
@TadaTeruki PR確認中ですが、lintが落ちてしまっているので、確認していただけますか！ |
| break; | ||
| } | ||
| // epsg | ||
| if localname.as_ref() == b"srsName" { |
There was a problem hiding this comment.
q)質問でした！
parse_geometry_propメソッドを修正しているということは、gml:boudedBy.gml:EnvelopのsrsNameを取得して一括で座標系を適用させる、ということではなく、全ての個別地物の座標系を取得して個別に適用させている、という認識で良いでしょうか？
There was a problem hiding this comment.
gml: EnvelopeのsrsNameは、全体というよりも、Envelopeそのものの空間参照系を指すattributionという認識でした。意味的には地物単体から取得するのが合理的と考えているのですが、いかがでしょうか...？
https://www.mlit.go.jp/plateaudocument
(11) エンベロープ型（gml:Envelope）
任意の次元で対向する角となる一対の位置（最小となる座標値と最大となる座標値）を用いて、矩形により範囲を定義する型。srsName属性とsrsDimension属性をもつことができる。srsName属性は、座標に使用される空間参照系を指定する。また、srsDimension属性は、座標の次元数を指定する。
There was a problem hiding this comment.
(なおデータのミスから各地物でepsgが間違っている・そもそも指定されていないものがある、という点も考慮が必要そうです。判断をお任せしてよろしいでしょうか。
There was a problem hiding this comment.
@TadaTeruki 同じく地物単体から座標系を取得し、個別に適用するのが合理的と思っています！
今でも、全ての地物に対して個別にtransformをかけているので、パフォーマンスにもさほど影響はないはずですし…
その上で、以下の点を考慮する必要がありますね。
- データのミスから各地物でepsgが間違っている
- データが悪いため、無視
- そもそも指定されていない
- デフォルト値として「gml:Envelope」を取得し、設定する
これでいきましょうか！
|
変換して表示できることは確認ずみ |
|
Lintの落ち、手元で 変更を新しいブランチに置きました おそらく自分の変更ではないため、念の為別ブランチに移しています。 |
|
また余談ですが、 同様の箇所が複数あり、diffが冗長となってしまいます。 |
修正お願いします！！ |
|
@TadaTeruki lgtmです！ |
TadaTeruki
force-pushed
the
fix/unf-coord-transformation
branch
from
41fc9a6
to
911a97a
Compare
Close #500
What I did（変更内容）
https://www.mlit.go.jp/plateaudocument/#toc9_06
PLATEAUのCityGMLデータにおいて、地下埋設物のジオメトリは「日本測地系2011における平面直角座標系と東京湾平均海面を基準とする標高の複合座標参照系」で記述されており、他のデータと異なっている
現在地下埋設物が配置されている竹芝モデルを対象とし、座標変換に対応した
(実際は実装の簡素化のため、平面直角座標系を6697に一旦変換してから、実装済みの変換を適用している)
各ジオメトリに紐付けられているEPSGコードをgml内から取得し適用している
Notes（連絡事項）
現在はPLATEAUの仕様に基づき、ファイル名を第一ソースとして座標系のEPSGを取得している (CityGMLの仕様には即していない)https://www.mlit.go.jp/plateaudocument/#toc9_07_04((下記はPRまでに行うfuture work))今後はgml内のタグのプロパティで取得することを視野に入れる実装はコメントアウトしている現在はこの点ではデータ側が不正確と考えられるため、応急処置としてこの方針としている