cocos / cocos-engine

Cocos simplifies game creation and distribution with Cocos Creator, a free, open-source, cross-platform game engine. Empowering millions of developers to create high-performance, engaging 2D/3D games and instant web entertainment.
https://www.cocos.com/en/creator
Other
8.87k stars 2.06k forks source link

Regarding post-processing issues with custom pipelines. #16059

Closed whaqzhzd closed 1 year ago

whaqzhzd commented 1 year ago

Cocos Creator version

3.8

System information

ALL

Issue description

There are 2 issues, they are:

  1. In the editor, when the global property of the post processing set on the screen is turned off, it is found that it cannot be turned off, and it is not persisted to the scene file. Restarting or switching scenes, the global property is always true.

  2. Although the post processing part is added to the passContext in line 178 of the builder, when rendering later, it does not filter out the post processing flows that do not belong to the camera that is currently being rendered. This will lead to rendering errors, as the camera incorrectly uses post processing that does not belong to itself. renderer

I'm not sure if setting-pass is used to filter out illegal post processing, but forcing it to return default here will cause the filtering to fail.

Relevant error log output

No response

Steps to reproduce

-

Minimal reproduction project

No response

2youyou2 commented 1 year ago
  1. https://github.com/cocos/cocos-engine/pull/16116 fixed
  2. I think the default setting is not the problem since setting-pass will filter the default setting enable && !!setting && setting.enabledInHierarchy , and the defaultSetting.enabledInHierarchy is false. Check it here

Did your inherited setting-pass call super.checkEnable like TAAPass?

If you still have problem, please uploading a test project for reproducing it.

whaqzhzd commented 1 year ago
  1. fixed serialize post process #16116 fixed
  2. I think the default setting is not the problem since setting-pass will filter the default setting enable && !!setting && setting.enabledInHierarchy , and the defaultSetting.enabledInHierarchy is false. Check it here

Did your inherited setting-pass call super.checkEnable like TAAPass?

If you still have problem, please uploading a test project for reproducing it.

Regarding the second issue, consider this scenario: Camera A has Bloom post-processing, Camera B has Blur post-processing. Object a is rendered by Camera A with Bloom. Object b is rendered by Camera B with Blur. Given Camera A has higher render priority than Camera B, Camera A renders first. In getSetting Camera A correctly gets Bloom. After Camera A rendering is done, now enter Camera B processing flow. First here passes traverses all post-processing passes, so passes here contain both Bloom and Blur. Then during filtering here setting... Because getSetting Bloom cannot be obtained here. But below default is forcibly returned. What would this lead to? It leads to the returned setting having a value, rather than the value obtained from getSetting based on the constructor. This is where the problem occurs. Have I expressed my meaning clearly?

whaqzhzd commented 1 year ago

Currently, we add post-processing by calling the insertPass function. The fundamental problem is actually because here obtains passes that contain all post-processing existing in the entire game. This array includes Bloom, Blur, and other passes added by default. Then checkEnable is called. It enters the getSetting function to check if the setting exists. The setting itself does not exist in the passContext.postProcess array. However, due to two points. First, here returns default. Thus causing here to become invalid. Second, here returns the processing of the entire game, rather than the current camera's processing. The combination of these two points produces this BUG.

2youyou2 commented 1 year ago

The default setting maybe confused, remove it is ok. Can this pr fixed your problem https://github.com/cocos/cocos-engine/pull/16116?

whaqzhzd commented 1 year ago

The default setting maybe confused, remove it is ok. Can this pr fixed your problem #16116?

You can make each camera have its own post processing by simply deleting the default. I've tested it in a custom engine.

whaqzhzd commented 1 year ago

There is still a bug regarding post processing at this stage. It will cause the Android to show a white screen. I still don't know the reason. I will make a demo for you

whaqzhzd commented 1 year ago

image

This demo is dual camera + dual post processing. One is bloom and the other is blur, any mixed colors rendered on screen. Everything works fine on web. After building Android, it will work normally for the first time or first few restarts. However, subsequent app launches will show a very unique white screen phenomenon. To make this demo work properly, make sure default is deleted in the custom engine. Similar to this PR . Otherwise the effect will be messy. No clue yet for the Android white screen issue. This demo can also be used to verify whether the default export causes the filtering to fail.

I've raised this issue on GitHub and look forward to your feedback on it. postProcessIssues.zip

2youyou2 commented 1 year ago

Which backend did you use? gles or vulkan?

whaqzhzd commented 1 year ago

Which backend did you use? gles or vulkan?

gles

2youyou2 commented 1 year ago

Build with default setting and restart the app for twenty times. Still can not reproduce this problem.

image

2youyou2 commented 1 year ago

Which device did you test? Can you try the vulkan backend, vulkan can be more stable.

whaqzhzd commented 1 year ago

Which device did you test? Can you try the vulkan backend, vulkan can be more stable.

Let me try ,My device is a Huawei Mate 10.

2youyou2 commented 1 year ago

I have tried vivo s9, hornor 30 and oneplus 11. Maybe it's a device related problem?

whaqzhzd commented 1 year ago

I have tried vivo s9, hornor 30 and oneplus 11. Maybe it's a device related problem?

I deleted the cache and it seems the problem no longer occurs, so let's just leave it at that. Thanks again Regarding this issue, I have no other questions left.

whaqzhzd commented 1 year ago

Regarding the issue I had in the previous demo where I needed to set the render texture size to be the same as the screen myself when using camera render textures, I suggest the engine help the user do this when the width and height of the render texture are both 0. Or you can add a setting for this in the render texture inspector. Of course it also works without adding this and letting users set it themselves, it's just more troublesome. This is just a suggestion.

image

2youyou2 commented 1 year ago

Thanks for the suggestion, we will consider it. Since the problem in this issue is solved, let's close it.