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

OBJ Sinkにアトラス化を付与したobj_atlas sinkの作成 #612

Closed
wants to merge 5 commits into from

Conversation

nokonoko1203
Copy link
Collaborator

@nokonoko1203 nokonoko1203 commented Aug 13, 2024

Description(変更内容)

  • 以下の事象に対応し、アトラス化されたテクスチャの出力と対応するOBJファイルを出力するように変更
    • UV座標をアトラスの座標に差し替え
    • mtlに書き込むテクスチャのパスと、アトラスの出力パスが不整合を修正
    • UV座標をピクセル座標に変換する際に、微妙な誤差や境界付近の取り扱い方により画像の範囲をはみ出してクロップしていた箇所を修正(atlas-packer)
  • アトラス化により処理時間が10倍以上になっているため、注意
    • 上記の理由などにより、本実装ではなくPoCです

Notes(連絡事項)

cargo run -- <input_path> --sink obj_atlas -o transform=use_texture -o split=true --output <output_path>

Copy link

coderabbitai bot commented Aug 13, 2024

Walkthrough

この変更は、画像の読み込み機能を削除し、テクスチャパッキング機能を強化したことに焦点を当てています。ObjAtlasSinkがテクスチャのキャッシュとパッカーを統合し、マテリアルの処理ロジックを簡素化。これにより、全体のパフォーマンスが向上し、マテリアル管理が効率的になりますが、マテリアルが見つからない場合のエラーハンドリングが簡素化されました。

Changes

ファイル 変更の概要
nusamai/src/sink/obj_atlas/material.rs load_image関数を削除し、不要なインポートを整理。これにより、画像の直接読み込み機能が失われました。
nusamai/src/sink/obj_atlas/mod.rs TextureCacheTexturePackerの導入により、テクスチャ管理機能を強化。新しい制御フローでテクスチャをキャッシュに挿入し、アトラスをエクスポートする機能を追加。
nusamai/src/sink/obj_atlas/obj_writer.rs write_obj関数からmaterialsパラメータを削除し、マテリアルキャッシュに依存するように変更。マテリアルの処理を簡素化。

Poem

🐰 うさぎの詩
画像は消えた、でも心配無用、
テクスチャの旅は、さらに楽しく、
新しいアトラスを作り、形を変え、
マテリアルの管理、速さが勝つ!
さあ、みんなで祝おう、コードの進化を! 🎉


Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@nokonoko1203 nokonoko1203 enabled auto-merge (squash) August 13, 2024 14:12
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (1)
nusamai/src/sink/obj_atlas/mod.rs (1)

378-380: テクスチャキャッシュの導入を確認しました。

TextureCacheが導入され、効率的なテクスチャ管理が可能になりました。キャッシュサイズが大きいため、メモリ使用量に注意が必要です。

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 87a9b61 and 2a63958.

Files selected for processing (3)
  • nusamai/src/sink/obj_atlas/material.rs (2 hunks)
  • nusamai/src/sink/obj_atlas/mod.rs (5 hunks)
  • nusamai/src/sink/obj_atlas/obj_writer.rs (4 hunks)
Additional comments not posted (5)
nusamai/src/sink/obj_atlas/material.rs (1)

3-3: 不要なコードとインポートの削除を確認しました。

load_image関数の削除により、コードが簡素化され、未使用のインポートが削除されました。これにより、コードの可読性とメンテナンス性が向上しています。

nusamai/src/sink/obj_atlas/obj_writer.rs (2)

100-103: テクスチャファイル名の取り扱いが改善されました。

テクスチャのファイル名が直接キャッシュに記録されるようになり、ファイルI/O操作が削減されました。これにより、効率が向上しています。


18-18: write_obj関数のインターフェースが簡素化されました。

materialsパラメータが削除され、material_cacheに依存するようになりました。これにより、関数のインターフェースが簡素化されましたが、材料が見つからない場合のエラーハンドリングが簡略化されているため、情報が失われる可能性があります。

Verification successful

write_obj関数のインターフェースの確認

write_obj関数は2つの異なる場所で定義されています。nusamai/src/sink/obj_atlas/obj_writer.rsの関数は、レビューコメントで指摘された新しいシグネチャに沿っていますが、nusamai/src/sink/obj/mod.rsの関数は異なるシグネチャを使用しています。このため、レビューコメントはobj_atlasバージョンにのみ適用されることを確認しました。

  • nusamai/src/sink/obj_atlas/obj_writer.rsでのwrite_objは新しいシグネチャに沿っています。
  • nusamai/src/sink/obj/mod.rsでのwrite_objは異なるシグネチャを使用しています。
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify usage of `write_obj` to ensure all calls align with the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type rust -A 5 $'write_obj'

Length of output: 1854

nusamai/src/sink/obj_atlas/mod.rs (2)

399-409: テクスチャパッカーの初期化が追加されました。

TexturePackerの初期化により、テクスチャのアトラス化が可能になり、処理が効率化されています。ただし、パッキングのパフォーマンスを監視することをお勧めします。


568-570: アトラスのエクスポート処理が追加されました。

packer.finalize()packer.export()が追加され、すべてのテクスチャが適切にパックされ、保存されるようになりました。これにより、アトラス化のプロセスが完結しています。

Comment on lines +54 to +56
// todo: Add error handling
println!("Material not found: {}", material_key);
continue;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

エラーハンドリングの改善を検討してください。

材料が見つからない場合に単にエラーメッセージを出力するだけではなく、詳細なエラーハンドリングを追加することを検討してください。これにより、デバッグが容易になります。

// 提案: ログに詳細な情報を追加し、エラーを処理する
eprintln!("Error: Material '{}' not found in cache. Please check the material definitions.", material_key);

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 125 lines in your changes missing coverage. Please review.

Files Patch % Lines
nusamai/src/sink/obj_atlas/mod.rs 0.00% 118 Missing ⚠️
nusamai/src/sink/obj_atlas/obj_writer.rs 0.00% 7 Missing ⚠️
Additional details and impacted files
Components Coverage Δ
GUI ∅ <ø> (∅)
Backend 62.51% <0.00%> (-0.44%) ⬇️
Libraries 89.82% <ø> (ø)

📢 Thoughts on this report? Let us know!

@satoshi7190
Copy link
Contributor

satoshi7190 commented Aug 14, 2024

@nokonoko1203
変換処理中にオーバーフローがatlas-packerのtexture.rsファイルの161行目で発生しています。
ご確認お願いします。

thread 'pipeline-sink' panicked at /Users/hoge/.cargo/git/checkouts/atlas-packer-0eceddb6ce829bab/c2c3e78/src/texture.rs:161:44:
attempt to subtract with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR nusamai::pipeline::runner > Sink thread panicked with message: attempt to subtract with overflow
 ERROR nusamai                   > Pipeline thread panicked: "Sink thread panicked with message: attempt to subtract with overflow"
 INFO  nusamai                   > Total processing time: 146.0530415s

@satoshi7190
Copy link
Contributor

@nokonoko1203 変換処理中にオーバーフローがatlas-packerのtexture.rsファイルの161行目で発生しています。 ご確認お願いします。

thread 'pipeline-sink' panicked at /Users/hoge/.cargo/git/checkouts/atlas-packer-0eceddb6ce829bab/c2c3e78/src/texture.rs:161:44:
attempt to subtract with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ERROR nusamai::pipeline::runner > Sink thread panicked with message: attempt to subtract with overflow
 ERROR nusamai                   > Pipeline thread panicked: "Sink thread panicked with message: attempt to subtract with overflow"
 INFO  nusamai                   > Total processing time: 146.0530415s

@nokonoko1203
こちら、クレートの更新をし忘れただけでした。すみません。

@satoshi7190
Copy link
Contributor

三角形でカットしてる部分が切れ目が見えちゃってるのもあるので、マージンをとってカットするのがよいのかも?(言うのは簡単ですが)
image

@nokonoko1203 nokonoko1203 marked this pull request as draft August 14, 2024 07:38
auto-merge was automatically disabled August 14, 2024 07:38

Pull request was converted to draft

satoshi7190 added a commit that referenced this pull request Aug 16, 2024
<!-- Close or Related Issues -->
Close #593 #311 #448 #303

### Description(変更内容)
<!-- Please describe the motivation behind this PR and the changes it
introduces. -->
<!-- 何のために、どのような変更をしますか? -->

- #612 を修正
- 以下の事象に対応し、アトラス化されたテクスチャの出力と対応するOBJファイルを出力するように変更
- UV座標をアトラスの座標に差し替え
- mtlに書き込むテクスチャのパスと、アトラスの出力パスが不整合を修正
- 不要なcrop処理を入れないように修正
- 全体的にマルチスレッド処理に変更
- 画像のキャッシュサイズを増加
- 一度切り出した`atlas-packer`を一旦nusamaiに戻す
  - 開発効率の問題

### Notes(連絡事項)
<!-- If manual testing is required, please describe the procedure. -->
<!-- 手動の動作確認が必要なら、手順を簡単に伝えてください。その他連絡事項など。 -->

```bash
cargo run -- <input_path> --sink obj -o transform=use_texture -o split=true --output <output_path>
```
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