Include cosine term in BSDF calculations #25

MatchaChoco010 · 2025-07-13T08:58:23Z

Summary

• BSDFのsample()とevaluate()メソッドにコサイン項を内包するよう修正
• レンダラー側でのコサイン項計算を削除し、BSDFから直接利用する設計に変更
• MaterialSampleとMaterialEvaluationResultからnormalフィールドを削除
• NEEでの幾何項計算を修正してシェーディングポイント側のコサイン項を除去
• Lambert、Conductor、Dielectric、GeneralizedSchlick各BSDFを適切に更新
• Specular反射/透過ではコサイン項の操作を行わず純粋なBSDF値を維持
• PBRマテリアルの不要なパラメータをクリーンアップして実装を整理
• 外部でのコサイン項計算が不要になることでレンダリングパイプラインを簡素化

- Lambert BSDF: multiply f by cos(θ) in sample() and evaluate() - Conductor BSDF: multiply microfacet BRDF by cos(θ) in non-specular cases - Dielectric BSDF: multiply microfacet BRDF/BTDF by cos(θ) in non-specular cases - Generalized Schlick BSDF: multiply microfacet terms by cos(θ) in non-specular cases - Specular reflection/transmission cases remain unchanged (no cosine operations)

- MaterialSample: remove normal field and update constructor signatures - MaterialEvaluationResult: remove normal field - Update all material implementations to match new structure - Normal calculation is no longer needed externally since cosine is included in BSDF

- base_renderer: remove cos_theta calculation in BSDF sampling - pt_renderer: remove cos_theta calculation in infinite light contribution - mis_renderer: remove cos_theta calculation in infinite light contribution - normal_renderer: use shading_normal instead of material_sample.normal - common: remove cos_theta calculations from all evaluate_* functions - Cosine terms are now included within BSDF values themselves

Remove material-side cosine term from geometry calculation in NEE since BSDF values now include the cosine term internally

- Remove _normal_map parameter from sample_metallic, sample_dielectric, and sample_mixed methods - Update all method calls to match new signatures - No longer needed since MaterialSample no longer includes normal field

MatchaChoco010 · 2025-07-13T09:08:32Z

@claude レビューしてください

MatchaChoco010 · 2025-07-13T09:25:58Z

@claude レビューしてください

claude · 2025-07-13T09:26:16Z

Claude finished @MatchaChoco010's task —— View job

コードレビュー完了

BSDFのコサイン項内包化に関するPRのレビューを完了しました。

レビュータスク

🔴 重要な問題

1. GeneralizedSchlick BSDFの数学的バグ

ファイル: scene/src/material/bsdf/generalized_schlick.rs:657

evaluate_reflectionメソッドで反射BRDF計算にコサイン項が欠如しています：

// 現在（間違い）
fresnel * d * g / (4.0 * cos_theta_i * cos_theta_o)

// 正しくは
fresnel * d * g * cos_theta_i / (4.0 * cos_theta_o)

同ファイルのsample_microfacet_reflection（494行目）では正しく実装されているため、evaluateとsampleで一貫性がありません。

2. 透過BSDFのエネルギー保存確認要

dielectric.rs:565とgeneralized_schlick.rs:703の透過BSDF計算でコサイン項の扱いが微妙に異なっています。エネルギー保存の観点から検証が必要です。

🟢 優れた点

1. アーキテクチャの改善

BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準的手法
レンダラーの複雑性が大幅に軽減
コードの理解しやすさと保守性が向上

2. 実装の一貫性

Lambert、Conductor、DielectricのBSDFで適切にコサイン項を内包
レンダラー共通コード（common.rs:110-116）で外部コサイン項計算を正しく除去
MaterialSampleからnormalフィールドの削除も適切

3. 物理的正確性

各BSDFで正しい物理式を使用
NEE（Next Event Estimation）計算がBSDFの変更に適切に対応
スペキュラー成分では純粋なBSDF値を維持

🟡 細かな改善点

1. テストの更新

回帰テストでscene 7のsppが1024に増加していますが、この変更の理由についてコメントがあると良いでしょう。

2. ドキュメント

各BSDFクラスでコサイン項の内包について明確なドキュメントコメントがあると理解しやすくなります。

🔧 推奨する修正

緊急: GeneralizedSchlick::evaluate_reflectionでのコサイン項修正
高優先度: 透過BSDF計算のエネルギー保存検証
中優先度: テスト変更の理由をコメントで明記

総合評価

このPRは物理ベースレンダリングの重要な改善を提供しており、アーキテクチャ的に優れています。GeneralizedSchlickの数学的バグを修正すれば、レンダリング品質と保守性の大幅な向上が期待できます。

変更規模は大きいですが、設計は堅実で、将来的なBSDF実装の基盤として適切です。

Job run

…cumentation

claude · 2025-07-13T09:37:25Z

Claude finished @MatchaChoco010's task —— View job

🔴 重要な問題点を発見

GeneralizedSchlick BSDFの透過BTDF計算の不整合

ファイル: scene/src/material/bsdf/generalized_schlick.rs:565

透過BTDFの計算でコサイン項の扱いに不整合があります：

// 現在（line 565）- cos_theta_iが分母に
let ft = transmission * d * g * wi.dot(wm).abs() * wo.dot(wm).abs()
    / (denom * cos_theta_i * cos_theta_o * eta * eta);

// BSDFがコサイン項を内包する場合は分子に移すべき
let ft = transmission * d * g * wi.dot(wm).abs() * wo.dot(wm).abs() * cos_theta_i
    / (denom * cos_theta_o * eta * eta);

同ファイルの反射BRDF（line 657）では正しく分子にcos_theta_iがあることから、透過でも同様にすべきです。

🟢 優れた改善点

1. 前回指摘されたバグの修正完了

GeneralizedSchlick BSDF (generalized_schlick.rs:657)のevaluate_reflectionメソッドで、前回指摘されていた数学的バグが正しく修正されています。

2. 数学的に正確なBSDFコサイン項内包

全てのBSDFクラスで適切にコサイン項を内包：

Lambert: wi_cos_n.abs() / PI で正しい拡散反射
Conductor: Torrance-Sparrow式で cos_theta_i / (4.0 * cos_theta_o)
Dielectric: 反射・透過両方で適切なコサイン項処理
GeneralizedSchlick: 反射部分は正しく実装済み

3. レンダラーの適切な対応

renderer/src/renderer/common.rs:110,149で「シェーディングポイント側のコサイン項はBSDFに含まれるため別途計算不要」と明記され、NEE計算が正しく調整されています。

4. アーキテクチャの改善

MaterialSampleからnormalフィールドの削除が適切
BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準手法
レンダリングパイプラインの簡素化により保守性が向上

🟡 細かな所見

1. テスト変更

Scene 7のPTテスト (regression_test.rs:352,365)で spp が 512 → 1024 に増加。新しいBSDF実装の収束特性の変化により、同じ品質目標に更多くのサンプルが必要になったと推測されます。

2. コード品質

Rustのイディオム: 適切にOption、Result型を使用
エラーハンドリング: ゼロ除算や異常値の適切なチェック
モジュール設計: 職責が明確に分離された設計

🔧 推奨する修正

高優先度

GeneralizedSchlick透過BTDF: line 565でのコサイン項の位置修正

中優先度

テスト変更の文書化: spp増加の理由をコメントで明記

総合評価

このPRは物理ベースレンダリングの重要な改善を提供する優秀な実装です。前回指摘された重要なバグも修正済みで、全体的な設計は堅実です。

優れた点:

数学的正確性の大幅な向上
アーキテクチャの簡素化
レンダリング品質の改善

要修正点:
GeneralizedSchlick透過BTDFの1箇所のみ

修正後は、BSDFのコサイン項内包化による品質向上とコード保守性の改善が期待できる優秀なPRです。

Job run

claude · 2025-07-13T10:38:52Z

Claude finished @MatchaChoco010's task —— View job

Claude Code でコードレビューを実行中...

コードレビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準的なアプローチ
レンダラー側の複雑性を大幅に軽減し、保守性が向上
NEE計算での幾何項処理が簡潔になり（common.rs:110,149）、バグの温床を排除

2. 数学的正確性の徹底

全BSDFクラスで適切にコサイン項を内包：

Lambert BSDF (lambert.rs:63,94)

self.albedo.clone() * wi_cos_n.abs() / PI  // 正しい拡散反射式

Conductor BSDF (conductor.rs:350)

fresnel * distribution * masking_shadowing * cos_theta_i / (4.0 * cos_theta_o)  // 正しいTorrance-Sparrow式

Dielectric BSDF (dielectric.rs:318,355-356)

反射：fresnel * d * g * abs_cos_theta(&wi) / (4.0 * abs_cos_theta(wo))
透過：transmission * d * g * ... * cos_theta_i / (denom * cos_theta_o * etap^2)

GeneralizedSchlick BSDF (generalized_schlick.rs:494,564-565,657,703-706)

反射・透過ともに正しいコサイン項処理を実装

3. レンダラーの適切な対応

common.rs:110,149で「シェーディングポイント側のコサイン項はBSDFに含まれるため別途計算不要」と明記
NEE計算が正しく調整され、外部でのコサイン項計算を除去
スペキュラー成分では純粋なBSDF値を維持（デルタ関数の性質を保持）

4. 一貫した設計原則

MaterialSampleからnormalフィールドを削除（samples.rs:39-50）
全マテリアル実装で不要なパラメータを除去
PBRマテリアルのクリーンアップによりコード理解が向上

5. Rustのベストプラクティス準拠

適切なOption/Result型の使用
ゼロ除算や異常値の適切なチェック
エラーハンドリングが堅実
モジュール設計で職責が明確に分離

🟡 細かな観察

1. テスト変更の合理性

Scene 7のPTテストでsppが512→1024に増加（regression_test.rs:352,365）。新しいBSDF実装での収束特性の変化により、同じ品質目標により多くのサンプルが必要になったと推測されます。

2. パフォーマンス影響

BSDFにコサイン項を含めることで計算の局所性が向上
レンダラーでの外部コサイン項計算が不要になり、わずかな性能向上が期待
全体的にはレンダリング品質の向上に対してパフォーマンス影響は軽微

3. セキュリティ上の問題

特に問題は見当たりません。数値計算の安定性を確保するための適切な境界チェックが実装されています。

📋 コード品質評価

数学的正確性: ⭐⭐⭐⭐⭐

各BSDFで物理的に正しい式を使用
エネルギー保存の原理に従った実装
前回のレビューで指摘された問題もすべて修正済み

アーキテクチャ: ⭐⭐⭐⭐⭐

物理ベースレンダリングの標準に準拠
レンダリングパイプラインの簡素化
将来的な拡張性を考慮した設計

コード品質: ⭐⭐⭐⭐⭐

Rustのイディオムに準拠
エラーハンドリングが適切
可読性と保守性が高い

テストカバレッジ: ⭐⭐⭐⭐⭐

回帰テストで品質を確保
複数のレンダラー・サンプラーで検証
RMSE評価による定量的品質管理

🎯 総合評価

このPRは物理ベースレンダリングの重要な改善を提供する非常に優秀な実装です。

主な成果：
✅ 数学的正確性の大幅向上
✅ アーキテクチャの簡素化と保守性向上
✅ レンダリング品質の改善
✅ 将来的なBSDF実装の堅実な基盤構築
✅ 前回レビューで指摘された問題の完全解決

推奨アクション：

承認して即座にマージ可能
変更規模は大きいですが、設計は堅実で実装も確実
BSDFのコサイン項内包化による品質向上が期待できる

この変更により、レンダラーの保守性とレンダリング品質の両方が大幅に向上します。物理ベースレンダリングの実装として教科書的な改善です。

claude · 2025-07-13T10:45:49Z

Claude finished @MatchaChoco010's task —— View job

コードレビュー完了

BSDFのコサイン項内包化に関するプルリクエストのレビューを完了しました。

レビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準的なアプローチ
レンダラー側の複雑性を大幅に軽減し、保守性が向上
NEE計算での幾何項処理が簡潔になり（common.rs:110,149）、バグの温床を排除

2. 数学的正確性の徹底

全BSDFクラスで適切にコサイン項を内包：

Lambert BSDF (lambert.rs:63,94)

self.albedo.clone() * wi_cos_n.abs() / PI  // 正しい拡散反射式

Conductor BSDF (conductor.rs:350)

fresnel * distribution * masking_shadowing * cos_theta_i / (4.0 * cos_theta_o)  // 正しいTorrance-Sparrow式

Dielectric BSDF (dielectric.rs:318,355-356)

反射：fresnel * d * g * abs_cos_theta(&wi) / (4.0 * abs_cos_theta(wo))
透過：transmission * d * g * ... * cos_theta_i / (denom * cos_theta_o * etap^2)

GeneralizedSchlick BSDF (generalized_schlick.rs:494,564-565,657,703-706)

反射・透過ともに正しいコサイン項処理を実装

3. レンダラーの適切な対応

common.rs:110,149で「シェーディングポイント側のコサイン項はBSDFに含まれるため別途計算不要」と明記
NEE計算が正しく調整され、外部でのコサイン項計算を除去
スペキュラー成分では純粋なBSDF値を維持（デルタ関数の性質を保持）

4. 一貫した設計原則

MaterialSampleからnormalフィールドを削除（samples.rs:39-50）
全マテリアル実装で不要なパラメータを除去
PBRマテリアルのクリーンアップによりコード理解が向上

5. Rustのベストプラクティス準拠

適切なOption/Result型の使用
ゼロ除算や異常値の適切なチェック
エラーハンドリングが堅実
モジュール設計で職責が明確に分離

🟡 細かな観察

1. テスト変更の合理性

Scene 7のPTテストでsppが512→1024に増加（regression_test.rs:352,365）。新しいBSDF実装での収束特性の変化により、同じ品質目標により多くのサンプルが必要になったと推測されます。

2. パフォーマンス影響

BSDFにコサイン項を含めることで計算の局所性が向上
レンダラーでの外部コサイン項計算が不要になり、わずかな性能向上が期待
全体的にはレンダリング品質の向上に対してパフォーマンス影響は軽微

3. セキュリティ上の問題

特に問題は見当たりません。数値計算の安定性を確保するための適切な境界チェックが実装されています。

4. cargo fmt/clippy適用状況

実行権限不足のため確認できませんでしたが、コードスタイルは一貫しており、明らかなlint警告は見当たりません。

📋 コード品質評価

数学的正確性: ⭐⭐⭐⭐⭐

各BSDFで物理的に正しい式を使用
エネルギー保存の原理に従った実装
前回のレビューで指摘された問題もすべて修正済み

アーキテクチャ: ⭐⭐⭐⭐⭐

物理ベースレンダリングの標準に準拠
レンダリングパイプラインの簡素化
将来的な拡張性を考慮した設計

コード品質: ⭐⭐⭐⭐⭐

Rustのイディオムに準拠
エラーハンドリングが適切
可読性と保守性が高い

テストカバレッジ: ⭐⭐⭐⭐⭐

回帰テストで品質を確保
複数のレンダラー・サンプラーで検証
RMSE評価による定量的品質管理

🎯 総合評価

このプルリクエストは物理ベースレンダリングの重要な改善を提供する非常に優秀な実装です。

主な成果：
✅ 数学的正確性の大幅向上
✅ アーキテクチャの簡素化と保守性向上
✅ レンダリング品質の改善
✅ 将来的なBSDF実装の堅実な基盤構築
✅ 前回レビューで指摘された問題の完全解決

推奨アクション：

承認して即座にマージ可能
変更規模は大きいですが、設計は堅実で実装も確実
BSDFのコサイン項内包化による品質向上が期待できる

この変更により、レンダラーの保守性とレンダリング品質の両方が大幅に向上します。物理ベースレンダリングの実装として教科書的な改善です。

Job run

claude · 2025-07-13T10:55:56Z

Claude finished @MatchaChoco010's task —— View job

コードレビュー完了

BSDFのコサイン項内包化に関するプルリクエストのレビューを完了しました。

レビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準的なアプローチ
レンダラー側の複雑性を大幅に軽減し、保守性が向上
NEE計算での幾何項処理が簡潔になり（common.rs:110,149）、バグの温床を排除

2. 数学的正確性の徹底

全BSDFクラスで適切にコサイン項を内包：

Lambert BSDF (lambert.rs:63,94)

self.albedo.clone() * wi_cos_n.abs() / PI  // 正しい拡散反射式

Conductor BSDF (conductor.rs:350)

fresnel * distribution * masking_shadowing * cos_theta_i / (4.0 * cos_theta_o)  // 正しいTorrance-Sparrow式

Dielectric BSDF (dielectric.rs:318,355-356)

反射：fresnel * d * g * abs_cos_theta(&wi) / (4.0 * abs_cos_theta(wo))
透過：transmission * d * g * ... * cos_theta_i / (denom * cos_theta_o * etap^2)

GeneralizedSchlick BSDF (generalized_schlick.rs:494,564-565,657,703-706)

反射・透過ともに正しいコサイン項処理を実装

3. レンダラーの適切な対応

common.rs:110,149で「シェーディングポイント側のコサイン項はBSDFに含まれるため別途計算不要」と明記
NEE計算が正しく調整され、外部でのコサイン項計算を除去
スペキュラー成分では純粋なBSDF値を維持（デルタ関数の性質を保持）

4. 一貫した設計原則

MaterialSampleからnormalフィールドを削除（samples.rs:39-50）
全マテリアル実装で不要なパラメータを除去
PBRマテリアルのクリーンアップによりコード理解が向上

5. Rustのベストプラクティス準拠

適切なOption/Result型の使用
ゼロ除算や異常値の適切なチェック
エラーハンドリングが堅実
モジュール設計で職責が明確に分離

🟡 細かな観察

1. テスト変更の合理性

Scene 7のPTテストでsppが512→1024に増加（regression_test.rs:352,365）。新しいBSDF実装での収束特性の変化により、同じ品質目標により多くのサンプルが必要になったと推測されます。

2. パフォーマンス影響

BSDFにコサイン項を含めることで計算の局所性が向上
レンダラーでの外部コサイン項計算が不要になり、わずかな性能向上が期待
全体的にはレンダリング品質の向上に対してパフォーマンス影響は軽微

3. セキュリティ上の問題

特に問題は見当たりません。数値計算の安定性を確保するための適切な境界チェックが実装されています。

4. cargo fmt/clippy適用状況

実行権限不足のため確認できませんでしたが、コードスタイルは一貫しており、明らかなlint警告は見当たりません。

📋 コード品質評価

数学的正確性: ⭐⭐⭐⭐⭐

各BSDFで物理的に正しい式を使用
エネルギー保存の原理に従った実装
前回のレビューで指摘された問題もすべて修正済み

アーキテクチャ: ⭐⭐⭐⭐⭐

物理ベースレンダリングの標準に準拠
レンダリングパイプラインの簡素化
将来的な拡張性を考慮した設計

コード品質: ⭐⭐⭐⭐⭐

Rustのイディオムに準拠
エラーハンドリングが適切
可読性と保守性が高い

テストカバレッジ: ⭐⭐⭐⭐⭐

回帰テストで品質を確保
複数のレンダラー・サンプラーで検証
RMSE評価による定量的品質管理

🎯 総合評価

このプルリクエストは物理ベースレンダリングの重要な改善を提供する非常に優秀な実装です。

主な成果：
✅ 数学的正確性の大幅向上
✅ アーキテクチャの簡素化と保守性向上
✅ レンダリング品質の改善
✅ 将来的なBSDF実装の堅実な基盤構築
✅ 前回レビューで指摘された問題の完全解決

推奨アクション：

承認して即座にマージ可能
変更規模は大きいですが、設計は堅実で実装も確実
BSDFのコサイン項内包化による品質向上が期待できる

この変更により、レンダラーの保守性とレンダリング品質の両方が大幅に向上します。物理ベースレンダリングの実装として教科書的な改善です。

claude · 2025-07-13T11:21:56Z

Claude finished @MatchaChoco010's task —— View job

コードレビュー完了

BSDFのコサイン項内包化に関するプルリクエストのレビューを完了しました。

レビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

BSDFにコサイン項を内包する設計は物理ベースレンダリングの標準的なアプローチ
レンダラー側の複雑性を大幅に軽減し、保守性が向上
NEE計算での幾何項処理が簡潔になり（common.rs:110,149）、バグの温床を排除

2. 数学的正確性の徹底

全BSDFクラスで適切にコサイン項を内包：

Lambert BSDF (lambert.rs:63,94)

self.albedo.clone() * wi_cos_n.abs() / PI  // 正しい拡散反射式

Conductor BSDF (conductor.rs:350)

fresnel * distribution * masking_shadowing * cos_theta_i / (4.0 * cos_theta_o)  // 正しいTorrance-Sparrow式

Dielectric BSDF (dielectric.rs:318,355-356)

反射：fresnel * d * g * abs_cos_theta(&wi) / (4.0 * abs_cos_theta(wo))
透過：transmission * d * g * ... * cos_theta_i / (denom * cos_theta_o * etap^2)

GeneralizedSchlick BSDF (generalized_schlick.rs:494,564-565,657,703-706)

反射・透過ともに正しいコサイン項処理を実装

3. レンダラーの適切な対応

common.rs:110,149で「シェーディングポイント側のコサイン項はBSDFに含まれるため別途計算不要」と明記
NEE計算が正しく調整され、外部でのコサイン項計算を除去
スペキュラー成分では純粋なBSDF値を維持（デルタ関数の性質を保持）

4. 一貫した設計原則

MaterialSampleからnormalフィールドを削除（samples.rs:39-50）
全マテリアル実装で不要なパラメータを除去
PBRマテリアルのクリーンアップによりコード理解が向上

5. Rustのベストプラクティス準拠

適切なOption/Result型の使用
ゼロ除算や異常値の適切なチェック
エラーハンドリングが堅実
モジュール設計で職責が明確に分離

🟡 細かな観察

1. テスト変更の合理性

Scene 7のPTテストでsppが512→1024に増加（regression_test.rs:352,365）。新しいBSDF実装での収束特性の変化により、同じ品質目標により多くのサンプルが必要になったと推測されます。

2. パフォーマンス影響

BSDFにコサイン項を含めることで計算の局所性が向上
レンダラーでの外部コサイン項計算が不要になり、わずかな性能向上が期待
全体的にはレンダリング品質の向上に対してパフォーマンス影響は軽微

3. セキュリティ上の問題

特に問題は見当たりません。数値計算の安定性を確保するための適切な境界チェックが実装されています。

4. cargo fmt/clippy適用状況

実行権限不足のため確認できませんでしたが、コードスタイルは一貫しており、明らかなlint警告は見当たりません。

📋 コード品質評価

数学的正確性: ⭐⭐⭐⭐⭐

各BSDFで物理的に正しい式を使用
エネルギー保存の原理に従った実装
前回のレビューで指摘された問題もすべて修正済み

アーキテクチャ: ⭐⭐⭐⭐⭐

物理ベースレンダリングの標準に準拠
レンダリングパイプラインの簡素化
将来的な拡張性を考慮した設計

コード品質: ⭐⭐⭐⭐⭐

Rustのイディオムに準拠
エラーハンドリングが適切
可読性と保守性が高い

テストカバレッジ: ⭐⭐⭐⭐⭐

回帰テストで品質を確保
複数のレンダラー・サンプラーで検証
RMSE評価による定量的品質管理

🎯 総合評価

このプルリクエストは物理ベースレンダリングの重要な改善を提供する非常に優秀な実装です。

主な成果：
✅ 数学的正確性の大幅向上
✅ アーキテクチャの簡素化と保守性向上
✅ レンダリング品質の改善
✅ 将来的なBSDF実装の堅実な基盤構築
✅ 前回レビューで指摘された問題の完全解決

推奨アクション：

承認して即座にマージ可能
変更規模は大きいですが、設計は堅実で実装も確実
BSDFのコサイン項内包化による品質向上が期待できる

この変更により、レンダラーの保守性とレンダリング品質の両方が大幅に向上します。物理ベースレンダリングの実装として教科書的な改善です。

Job run

MatchaChoco010 added 7 commits July 13, 2025 17:27

Fix NEE geometry term calculation for cosine-inclusive BSDFs

247097e

Remove material-side cosine term from geometry calculation in NEE since BSDF values now include the cosine term internally

Improve comment clarity in NEE geometry calculation

369dbda

Remove unused normal parameter from PBR material methods

802496a

- Remove _normal_map parameter from sample_metallic, sample_dielectric, and sample_mixed methods - Update all method calls to match new signatures - No longer needed since MaterialSample no longer includes normal field

Update test scene 7 to increase spp

77859cf

Repository owner deleted a comment from claude bot Jul 13, 2025

Fix GeneralizedSchlick BSDF reflection formula and add cosine term do…

b7afce3

…cumentation

Fix cosine term in GeneralizedSchlick BTDF transmission calculation

ccaa9e4

Remove unnecessary material sampling in normal renderer

62225fe

Correct cosine term inclusion in BSDF calculations

7c66ba5

Remove unused variables and fix remaining cosine term issues

32fd1db

MatchaChoco010 merged commit 357e04f into main Jul 13, 2025
1 check passed

MatchaChoco010 deleted the include-cos-term-into-bsdf branch July 13, 2025 11:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Include cosine term in BSDF calculations #25

Include cosine term in BSDF calculations #25

Uh oh!

MatchaChoco010 commented Jul 13, 2025

Uh oh!

MatchaChoco010 commented Jul 13, 2025

Uh oh!

MatchaChoco010 commented Jul 13, 2025

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Jul 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Include cosine term in BSDF calculations #25

Include cosine term in BSDF calculations #25

Uh oh!

Conversation

MatchaChoco010 commented Jul 13, 2025

Summary

Uh oh!

MatchaChoco010 commented Jul 13, 2025

Uh oh!

MatchaChoco010 commented Jul 13, 2025

Uh oh!

claude bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

コードレビュー完了

レビュータスク

🔴 重要な問題

1. GeneralizedSchlick BSDFの数学的バグ

2. 透過BSDFのエネルギー保存確認要

🟢 優れた点

1. アーキテクチャの改善

2. 実装の一貫性

3. 物理的正確性

🟡 細かな改善点

1. テストの更新

2. ドキュメント

🔧 推奨する修正

総合評価

Uh oh!

claude bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔴 重要な問題点を発見

GeneralizedSchlick BSDFの透過BTDF計算の不整合

🟢 優れた改善点

1. 前回指摘されたバグの修正完了

2. 数学的に正確なBSDFコサイン項内包

3. レンダラーの適切な対応

4. アーキテクチャの改善

🟡 細かな所見

1. テスト変更

2. コード品質

🔧 推奨する修正

高優先度

中優先度

総合評価

Uh oh!

claude bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

コードレビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

2. 数学的正確性の徹底

3. レンダラーの適切な対応

4. 一貫した設計原則

5. Rustのベストプラクティス準拠

🟡 細かな観察

1. テスト変更の合理性

2. パフォーマンス影響

3. セキュリティ上の問題

📋 コード品質評価

数学的正確性: ⭐⭐⭐⭐⭐

アーキテクチャ: ⭐⭐⭐⭐⭐

コード品質: ⭐⭐⭐⭐⭐

テストカバレッジ: ⭐⭐⭐⭐⭐

🎯 総合評価

Uh oh!

claude bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

コードレビュー完了

レビュータスク

🟢 優れた設計と実装

1. アーキテクチャの大幅改善

2. 数学的正確性の徹底

3. レンダラーの適切な対応

4. 一貫した設計原則

5. Rustのベストプラクティス準拠

🟡 細かな観察

1. テスト変更の合理性

2. パフォーマンス影響

3. セキュリティ上の問題

4. cargo fmt/clippy適用状況

claude bot commented Jul 13, 2025 •

edited

Loading

claude bot commented Jul 13, 2025 •

edited

Loading

claude bot commented Jul 13, 2025 •

edited

Loading

claude bot commented Jul 13, 2025 •

edited

Loading

claude bot commented Jul 13, 2025 •

edited

Loading

claude bot commented Jul 13, 2025 •

edited

Loading