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

fix render window attach camera and resize logic #17414

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

star-e
Copy link
Contributor

@star-e star-e commented Jul 25, 2024

When a new camera is attached to the render window, the render window will be set resized. The custom pipeline will update all associated resources.

Fix window resize logic, it will be resized multiple times with different cameras. User should reuse render window's resource and avoid collision.

Changelog


Continuous Integration

This pull request:

  • needs automatic test cases check.

    Manual trigger with @cocos-robot run test cases afterward.

  • does not change any runtime related code or build configuration

    If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.


Compatibility Check

This pull request:

  • changes public API, and have ensured backward compatibility with deprecated features.
  • affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
  • affects file structure of the build package or build configuration which requires user project upgrade.
  • introduces breaking changes, please list all changes, affected features and the scope of violation.

Greptile Summary

The pull request improves the handling of render window resizing and camera attachment logic to prevent multiple resize operations and resource collisions.

  • Added _isResized flag in cocos/render-scene/core/render-window.ts to track window resize state.
  • Introduced _resizedWindows array in cocos/rendering/custom/framework.ts to manage resized windows and reset flags post-processing.
  • Updated attachCamera method in cocos/render-scene/core/render-window.ts to set _isResized flag when a new camera is attached.
  • Enhanced dispatchResizeEvents function in cocos/rendering/custom/framework.ts to handle multiple cameras and avoid redundant resizing.
  • Modified native/cocos/scene/RenderWindow.cpp to align with the new resize and camera attachment logic.

@star-e star-e requested a review from minggo July 25, 2024 09:52
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

Interface Check Report

This pull request does not change any public interfaces !

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

export function dispatchResizeEvents (cameras: Camera[], builder: PipelineBuilder, ppl: BasicPipeline): void {
if (!builder.windowResize) {
// No game window resize handler defined.
// Following old prodecure, do nothing
return;
}

// Resize all windows.
// Notice: A window might be resized multiple times with different cameras.
// User should avoid resource collision between different cameras.
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen if two or more cameras have have the same size?

Copy link
Contributor Author

@star-e star-e Jul 25, 2024

Choose a reason for hiding this comment

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

The custom pipeline resize logic of the specific render window will be called multiple times with different cameras.

This is by design, in order to:

  1. Reduce memory usage by reusing resources of the render window.
  2. Share render targets between different cameras.

User can also use camera name/index, to avoid collision if they really want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cameras' sizes are the same of the render window.

@star-e star-e merged commit 27221f5 into cocos:v3.8.4 Jul 25, 2024
23 checks passed
@star-e star-e deleted the v3.8.4-fix-camera branch July 25, 2024 14:48
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