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

Blur Node output resolution doesn't adapt to Visual Compositor's Resolution changes #36

Open
ShiinaRinne opened this issue Jan 4, 2025 · 4 comments
Assignees

Comments

@ShiinaRinne
Copy link

Describe the bug
The output resolution of the Blur Node doesn't automatically adjust when the Resolution of the Visual Compositor component changes. This leads to rendering errors when the Resolution is modified, requiring the Blur Node to be deleted and rebuilt.

To Reproduce
Steps to reproduce the behavior:

  1. Create a Blur Node with the Visual Compositor's Resolution set to 1920x1080.
  2. Change the Visual Compositor's Resolution to 2560x1440.

Screenshots
image

Unity (please complete the following information):

  • Unity2021-Unity6000
  • URP
  • VisualCompositor 0.30.7

Additional context
Currently, the m_size variable in the Blur Node is only initialized to Vector2Int.Zero when m_input is null.
It's then set to Vector2Int(m_input.width, m_input.height).

However, when m_input's width and height change, m_size isn't updated accordingly.

I've implemented a temporary fix for this issue:

// Runtime/Scripts/Nodes/BlurNode.cs, line 49
if (m_size == Vector2Int.zero || m_size != new Vector2Int(m_input.width, m_input.height)) {
   m_size.SetValue(new Vector2Int(m_input.width, m_input.height));
}

While frequent resolution changes may not be common, I believe this adjustment would be beneficial for flexibility and user experience. It's particularly useful when testing at lower resolutions (e.g., 1920x1080) but needing to output at higher resolutions (4K or 8K) to avoid aliasing issues.

I haven't verified if other nodes have similar issues, but it might be worth investigating.


Additionally, I've noticed a change in VisualCompositor 0.30.6 regarding the namespace location for RenderGraphModule in Unity 6. Specifically, in Runtime\Scripts\Nodes\ColorGrading\URP\ColorGradingLutPass.cs, the line:

using UnityEngine.Experimental.Rendering.RenderGraphModule;

was updated to:

using UnityEngine.Rendering.RenderGraphModule;

This change has resulted in compatibility issues for users with older Unity versions, requiring them to manually downgrade to VisualCompositor 0.30.5.

I believe it might be beneficial to use a preprocessor directive like #if UNITY_VERSION_2023_OR_NEWER to handle this change. This approach would allow for better version compatibility across different Unity releases.

Would it be possible to consider implementing this change? It could greatly improve the user experience for those working with various Unity versions.

Thank you for your consideration.

@sindharta-tanuwijaya
Copy link
Contributor

Hello, and thank you always for your valuable feedback.

Due to the company reset earlier last year, we at Unity have unfortunately had to deprioritize the development of the AnimeToolbox packages.

Under normal circumstances, this would limit our ability to implement new features or address existing bugs. However, the issues you've identified seem manageable, especially with the example code you provided. I believe I can allocate some time to release an updated version of VisualCompositor.

We truly appreciate your understanding and continued support.

@ShiinaRinne
Copy link
Author

I’m not sure about the criteria Unity uses for deciding whether or not to host code on GitHub, but if you can open-source VisualCompositor (host it on GitHub), I’d be happy to contribute and help maintain it within my capabilities, though of course, it would still require your review.

For example, in the version I use, I’ve added simple SNN Filter, Kuwahara Filter and Color Transfer, but since much of the code is internal, I can’t release it as a plugin and can only modify the original codebase.

I truly respect the work Unity is doing, and I wish you and the team continued success as you navigate through these changes. I look forward to any future updates or contributions I can make.

Thank you again for your time and support~

@sindharta-tanuwijaya
Copy link
Contributor

I have released [email protected], which addresses several bugs you mentioned earlier.

As you wrote above, the size of BlurNode should correspond to the size set in the VisualCompositor component by default, and the above patch release addresses this bug.

I can understand the desire for the BlurNode's size to automatically match its input size, but that's probably something that we should do using a separate UI field, which could be considered for a future release, once we have resources to focus on developing this package further.

I’ve added simple SNN Filter, Kuwahara Filter and Color Transfer, but since much of the code is internal, I can’t release it as a plugin and can only modify the original codebase.

Would love to see you share your work with the community ! 😊
We have support for creating custom nodes, which allows you to add them directly to your project without the need to modify the VisualCompositor package itself.

Do you think you could adapt your nodes using this method, or are there specific internal functions you need access to for this purpose?

@ShiinaRinne
Copy link
Author

Many thanks!

Do you think you could adapt your nodes using this method, or are there specific internal functions you need access to for this purpose?

I apologize, but since a considerable amount of time has passed, I'm having trouble recalling the exact modifications I made. Unfortunately, I can't locate the original project backup at the moment.

However, if my memory serves me correctly, I believe the issue was related to the ScreenShaderNode.
This node is incredibly convenient for creating other nodes, but it's marked as internal, which prevents its use in custom nodes.

To resolve this, I changed this class and other related classes to public.
Despite my uncertainty about whether this modification might have other unintended consequences

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

No branches or pull requests

2 participants