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 the issue where post-processing modifications after rendering are ineffective. #17586

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

GengineJS
Copy link
Contributor

@GengineJS GengineJS commented Aug 28, 2024

Greptile Summary

This pull request modifies the BuiltinPipelineSettings class in the default render pipeline, specifically affecting the bloom intensity property.

  • Removed @Property decorator for 'bloomIntensity' setter in editor/assets/default_renderpipeline/builtin-pipeline-settings.ts
  • Potential impact on bloom intensity property visibility and editability in the editor interface
  • Getter and setter methods for bloomIntensity remain intact, preserving programmatic access

@GengineJS GengineJS requested a review from star-e August 28, 2024 11:45
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.

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

Comment on lines 475 to +485
updateBuffer (bindId: number, vals: number[], layout: string, setData: DescriptorSetData): void {
const descriptorSet = setData.descriptorSet!;
const buffer = this.getCurrentBuffer();
const uniformKey = `${layout}${bindId}${this.currBuffIdx}`;
let currUniform = uniformMap.get(uniformKey);
const currHash = numsHash(vals);
let currUniform = uniformMap.get(buffer);
if (!currUniform) {
currUniform = new Float32Array(vals);
uniformMap.set(uniformKey, currUniform);
}
const destHash = buffHashMap.get(buffer);
if (destHash !== currHash) {
currUniform.set(vals);
buffer.update(currUniform);
bindGlobalDesc(descriptorSet, bindId, buffer);
buffHashMap.set(buffer, currHash);
uniformMap.set(buffer, currUniform);
}
currUniform.set(vals);
buffer.update(currUniform);
bindGlobalDesc(descriptorSet, bindId, buffer);
Copy link

Choose a reason for hiding this comment

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

style: Buffer is now always updated, which may affect performance but ensures consistency

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

@GengineJS GengineJS requested a review from minggo August 29, 2024 02:11
@minggo minggo merged commit 9a4693e into cocos:v3.8.4 Aug 29, 2024
10 checks passed
yoki0805 pushed a commit to yoki0805/cocos-engine that referenced this pull request Sep 5, 2024
… ineffective. (cocos#17586)

* Fix the issue where post-processing modifications after rendering are ineffective.

* Removes the intensity edit slot for bloom effect
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