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

RenderGraph support for Distortion #111

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

CA-Tatami
Copy link
Contributor

@CA-Tatami CA-Tatami commented Mar 13, 2025

DistortionをRenderGraphに対応させました。
WindowsのDX11のUnity 2021,2022, 6000環境にてAverage Testを実行済みです。

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds RenderGraph support for the distortion feature, updating both new RenderGraph passes and the legacy passes to improve rendering performance and integration. Key changes include:

  • New RenderGraph passes for distorted UV buffer and applying distortion.
  • Updates to existing ScreenSpaceDistortion and legacy passes with minor formatting and copyright updates.
  • Conversion of some classes from sealed to partial to support RenderGraph scenarios.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Assets/Nova/Runtime/Core/Scripts/DistortionContextItem_RenderGraph.cs Introduces a RenderGraph context item for distortion.
Assets/Nova/Runtime/Core/Scripts/DistortedUvBufferPass_RenderGraph.cs Implements a new RenderGraph pass for the distorted UV buffer.
Assets/Nova/Runtime/Core/Scripts/ApplyDistortionPass_RenderGraph.cs Implements a new RenderGraph pass for applying distortion using the context item.
Assets/Nova/Runtime/Core/Scripts/ScreenSpaceDistortion.cs Updates shader lookup and minor formatting adjustments.
Assets/Nova/Runtime/Core/Scripts/DistortedUvBufferPass.cs Updates legacy pass implementation; removes the sealed modifier.
Assets/Nova/Runtime/Core/Scripts/ApplyDistortionPass.cs Updates legacy pass implementation; converts to partial class and simplifies conditionals.
Comments suppressed due to low confidence (2)

Assets/Nova/Runtime/Core/Scripts/ApplyDistortionPass_RenderGraph.cs:26

  • The error message here could be more descriptive about the required dependency on the DistortedUvBufferPass. Consider clarifying the execution order dependency to help diagnose setup issues.
Debug.LogError("[NOVA] Cannot execute ApplyDistortionPass. DistortedUvBufferPass must be executed.");

Assets/Nova/Runtime/Core/Scripts/DistortedUvBufferPass.cs:11

  • The removal of the 'sealed' modifier changes the class's extensibility. Confirm that this change is intentional as it may affect inheritance and class design.
public class DistortedUvBufferPass : ScriptableRenderPass
@@ -8,7 +8,7 @@

namespace Nova.Runtime.Core.Scripts
{
public sealed class DistortedUvBufferPass : ScriptableRenderPass
public class DistortedUvBufferPass : ScriptableRenderPass
Copy link
Contributor

Choose a reason for hiding this comment

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

partialがないのでコンパイルエラーが起きています

Copy link
Contributor

Choose a reason for hiding this comment

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

Unity 6000.0.40f1にて添付している画像のコンパイルエラーが起きています。
エラー

Copy link
Contributor

Choose a reason for hiding this comment

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

プロジェクトを開くときに古いAPIを使っています。置き換えますか?というダイアログでNoを選ぶと確認できると思います。

Copy link
Contributor

Choose a reason for hiding this comment

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

追加でRender Graphを有効にしてApplyDistortionPass.cs、ApplyDistortionPass.cs、ScreenSpaceDistortion.cs再インポートしてコンパイルを走らせると添付の画像の警告が出てきます。
ObsoleteのAPIを利用しているために出ている警告です。

Siriusに同様の警告を抑止している処理があるので、そちらを参考に実装してみてください。

警告

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.

3 participants