テクスチャのスケーリング (#586) #623
テクスチャのスケーリング (#586) #623
Conversation
Walkthroughこの変更は、 Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add 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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- nusamai/src/sink/cesiumtiles/mod.rs (3 hunks)
Additional comments not posted (3)
nusamai/src/sink/cesiumtiles/mod.rs (3)
49-50: 定数MAX_PIXEL_PER_DISTANCEの追加についてこの定数はテクスチャのピクセル密度を制限するために使用されます。コード内での使用方法とその値が適切かどうかを確認してください。
この変更は承認されます。
520-567: テクスチャのダウンサンプリング計算についてこのコードブロックでは、各頂点のUV座標からピクセル座標を計算し、それを使用してユークリッド距離とテクスチャ距離を比較しています。計算された
pixel_per_distanceを使用して、テクスチャのダウンサンプリング比を決定しています。このロジックはPRの目的に合致しており、実装も適切ですが、パフォーマンスの観点から最適化の余地があるかもしれません。この変更は承認されますが、パフォーマンスの最適化を検討してください。
733-740: 関数apply_downsample_factorの変更についてこの関数はズームレベルに基づいてダウンサンプリングの係数を計算し、新たに追加された
downsample_scaleパラメータを使用して最終的なダウンサンプリング係数を決定します。この変更はテクスチャの解像度を適切に管理するために重要です。この変更は承認されます。
| // WARN: This function has an equivalent in `atlas-packer/src/texture.rs`. | ||
| fn uv_to_pixel_coords(uv_coords: &[(f64, f64)], width: u32, height: u32) -> Vec<(u32, u32)> { | ||
| uv_coords | ||
| .iter() | ||
| .map(|(u, v)| { | ||
| ( | ||
| (u.clamp(0.0, 1.0) * width as f64).min(width as f64 - 1.0) as u32, | ||
| ((1.0 - v.clamp(0.0, 1.0)) * height as f64).min(height as f64 - 1.0) as u32, | ||
| ) | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
関数 uv_to_pixel_coords の重複について
この関数はUV座標をピクセル座標に変換する機能を提供しますが、atlas-packer に同様の機能が存在しているとの警告があります。この重複が意図的である場合は問題ありませんが、将来的にメンテナンスの負担を減らすために一元化を検討することをお勧めします。
重複するコードの削減を検討してください。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
📢 Thoughts on this report? Let us know! |
| const MAX_PIXEL_PER_DISTANCE: f64 = 15.0; | ||
|
|
||
| // WARN: This function has an equivalent in `atlas-packer/src/texture.rs`. | ||
| fn uv_to_pixel_coords(uv_coords: &[(f64, f64)], width: u32, height: u32) -> Vec<(u32, u32)> { |
There was a problem hiding this comment.
atlas-packer側の実装を変更してまでDRYを貫く必要性はないと感じているため、ここでは一旦重複した状態のままでコミットしています
良いと思います！
| @@ -46,6 +46,21 @@ use crate::{ | |||
| }; | |||
| use utils::calculate_normal; | |||
|
|
|||
| const MAX_PIXEL_PER_DISTANCE: f64 = 15.0; | |||
|
|
|||
There was a problem hiding this comment.
30くらいに設定しないと、何が記載されている看板か不明なレベルまで解像度が落ちてしまったので、暫定的に30にしました。
より良い数値は見つけていきましょう！
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/sink/cesiumtiles/mod.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- nusamai/src/sink/cesiumtiles/mod.rs
Close #586
What I did（変更内容）
各ポリゴンについて、一定の距離に対するテクスチャのピクセルの数を制限し、制限を超える場合はテクスチャをスケーリングします。
(文章が長くなるので、距離ごとのピクセル数 = PD とします)
具体的な処理を述べると
Notes（連絡事項）
uv_to_pixel_coords)を追加しているのですが、atlas-packer側の実装と重複しています。ここは仕様が頻りに変わる処理ではなく、atlas-packer側の実装を変更してまでDRYを貫く必要性はないと感じているため、ここでは一旦重複した状態のままでコミットしています。MAX_PIXEL_PER_DISTANCEとして宣言しています(=15.0)。