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

Refactor PostProcessParameter to strong type #2487

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions e2e/case/postProcess-customPass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Engine,
Material,
PostProcess,
PostProcessEffectFloatParameter,
PostProcessPass,
PostProcessPassEvent,
RenderTarget,
Expand Down Expand Up @@ -43,9 +44,7 @@ void main() {
class CustomPass extends PostProcessPass {
private _blitMaterial: Material;

set intensity(value) {
this._blitMaterial.shaderData.setFloat("intensity", value);
}
intensity = new PostProcessEffectFloatParameter(0.7, 0, 1);

constructor(engine: Engine) {
super(engine);
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/postProcess/PostProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,12 @@ export class PostProcess extends Component {
override _onDisableInScene() {
this.scene.postProcessManager._removePostProcess(this);
}

/**
* @inheritdoc
*/
override _onDestroy(): void {
super._onDestroy();
this._effects.length = 0;
}
Comment on lines +130 to +137
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure all effect resources are properly released in _onDestroy.

Clearing _effects is a good step. Also consider calling a destroy / cleanup method on each effect if they hold resources (buffers, textures, etc.). That ensures no memory leaks persist within the engine or GPU resources.

}
231 changes: 172 additions & 59 deletions packages/core/src/postProcess/PostProcessEffectParameter.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import { Color, MathUtil, Vector2, Vector3, Vector4 } from "@galacean/engine-math";
import { Texture } from "../texture/Texture";
import { Texture } from "../texture";

/**
* Represents a parameter of a post process effect.
* @remarks
* The parameter will be mixed to a final value and be used in post process manager.
*/
export class PostProcessEffectParameter<T extends Number | Boolean | Color | Vector2 | Vector3 | Vector4 | Texture> {
export abstract class PostProcessEffectParameter<T> {
/**
* Whether the parameter is enabled.
*/
enabled = true;

private _value: T;
private _needLerp = false;
private _min?: number;
private _max?: number;
readonly type;

protected _needLerp = false;
protected _value: T;

/**
* The value of the parameter.
Expand All @@ -25,71 +25,184 @@ export class PostProcessEffectParameter<T extends Number | Boolean | Color | Vec
}

set value(value: T) {
if (value?.constructor === Number) {
this._value = <T>(<unknown>MathUtil.clamp(<number>value, this._min, this._max));
} else {
this._value = value;
this._value = value;
}

constructor(type, value: T, needLerp = false) {
this.type = type;
this._needLerp = needLerp;
this._value = value;
}

_lerp(to: T, factor: number) {
if (factor > 0) {
this.value = to;
}
}
}

/**
* Represents a float parameter of a post process effect.
*/
export class PostProcessEffectFloatParameter extends PostProcessEffectParameter<number> {
override get value(): number {
return this._value;
}

constructor(value: Exclude<T, number>, needLerp?: boolean);
constructor(value: Exclude<T, Boolean | Color | Vector2 | Vector3 | Vector4 | Texture>, needLerp?: boolean);
override set value(v: number) {
this._value = MathUtil.clamp(v, this._min, this._max);
}

/**
* Create a new float parameter.
* @param value - The default value of the parameter
* @param _min - The minimum value of the parameter, default is Number.NEGATIVE_INFINITY
* @param _max - The maximum value of the parameter, default is Number.POSITIVE_INFINITY
* @param needLerp - Whether the parameter needs to be lerp, default is true
*/
constructor(
value: Exclude<T, Boolean | Color | Vector2 | Vector3 | Vector4 | Texture>,
min?: number,
max?: number,
needLerp?: boolean
);

constructor(value: T, needLerpOrMin?: boolean | number, max?: number, needLerp?: boolean) {
if (typeof value === "number") {
if (typeof needLerpOrMin === "boolean") {
this._needLerp = needLerpOrMin;
this._min = Number.NEGATIVE_INFINITY;
this._max = Number.POSITIVE_INFINITY;
} else if (typeof needLerpOrMin === "number") {
this._min = needLerpOrMin;
this._max = max ?? Number.POSITIVE_INFINITY;
this._needLerp = needLerp ?? false;
} else if (needLerpOrMin == undefined) {
this._min = Number.NEGATIVE_INFINITY;
this._max = Number.POSITIVE_INFINITY;
}
value: number,
private _min = Number.NEGATIVE_INFINITY,
private _max = Number.POSITIVE_INFINITY,
needLerp = true
) {
super(Number, value, needLerp);
this.value = value;
}

override _lerp(to: number, factor: number) {
if (this._needLerp) {
this.value = MathUtil.lerp(this.value, to, factor);
} else {
this._needLerp = <boolean>needLerpOrMin ?? false;
super._lerp(to, factor);
}
}
}

this.value = value;
/**
* Represents a boolean parameter of a post process effect.
*/
export class PostProcessEffectBoolParameter extends PostProcessEffectParameter<boolean> {
/**
* Create a new boolean parameter.
* @param value - The default value of the parameter
*/
constructor(value: boolean) {
super(Boolean, value, false);
}
}

/**
* Represents a texture parameter of a post process effect.
*/
export class PostProcessEffectTextureParameter extends PostProcessEffectParameter<Texture> {
/**
* @internal
* Create a new texture parameter.
* @param value - The default texture of the parameter
*/
_lerp(to: T, factor: number) {
constructor(value: Texture) {
super(Texture, value, false);
}
}

/**
* Represents a color parameter of a post process effect.
*/
export class PostProcessEffectColorParameter extends PostProcessEffectParameter<Color> {
/**
* Create a new color parameter.
* @param value - The default color of the parameter
* @param needLerp - Whether the parameter needs to be lerp, default is true
*/
constructor(value: Color, needLerp = true) {
super(Color, value, needLerp);
}

override _lerp(to: Color, factor: number) {
if (this._needLerp) {
switch (this.value?.constructor) {
case Number:
this.value = <T>(<unknown>MathUtil.lerp(<number>this.value, <number>to, factor));
break;
case Color:
Color.lerp(<Color>this.value, <Color>to, factor, <Color>this.value);
break;
case Vector2:
Vector2.lerp(<Vector2>this.value, <Vector2>to, factor, <Vector2>this.value);
break;
case Vector3:
Vector3.lerp(<Vector3>this.value, <Vector3>to, factor, <Vector3>this.value);
break;
case Vector4:
Vector4.lerp(<Vector4>this.value, <Vector4>to, factor, <Vector4>this.value);
break;
default:
if (factor > 0) {
this.value = to;
}
}
} else if (factor > 0) {
this.value = to;
Color.lerp(this.value, to, factor, this.value);
} else {
super._lerp(to, factor);
}
}
}

/**
* Represents a vector2 parameter of a post process effect.
*/
export class PostProcessEffectVector2Parameter extends PostProcessEffectParameter<Vector2> {
/**
* Create a new vector2 parameter.
* @param value - The default vector2 of the parameter
* @param needLerp - Whether the parameter needs to be lerp, default is true
*/
constructor(value: Vector2, needLerp = true) {
super(Vector2, value, needLerp);
}

override _lerp(to: Vector2, factor: number) {
if (this._needLerp) {
Vector2.lerp(this.value, to, factor, this.value);
} else {
super._lerp(to, factor);
}
}
}

/**
* Represents a vector3 parameter of a post process effect.
*/
export class PostProcessEffectVector3Parameter extends PostProcessEffectParameter<Vector3> {
/**
* Create a new vector3 parameter.
* @param value - The default vector3 of the parameter
* @param needLerp - Whether the parameter needs to be lerp, default is true
*/
constructor(value: Vector3, needLerp = true) {
super(Vector3, value, needLerp);
}

override _lerp(to: Vector3, factor: number) {
if (this._needLerp) {
Vector3.lerp(this.value, to, factor, this.value);
} else {
super._lerp(to, factor);
}
}
}

/**
* Represents a vector4 parameter of a post process effect.
*/
export class PostProcessEffectVector4Parameter extends PostProcessEffectParameter<Vector4> {
/**
* Create a new vector4 parameter.
* @param value - The default vector4 of the parameter
* @param needLerp - Whether the parameter needs to be lerp, default is true
*/
constructor(value: Vector4, needLerp = true) {
super(Vector4, value, needLerp);
}

override _lerp(to: Vector4, factor: number) {
if (this._needLerp) {
Vector4.lerp(this.value, to, factor, this.value);
} else {
super._lerp(to, factor);
}
}
}

/**
* Represents a enum parameter of a post process effect.
*/
export class PostProcessEffectEnumParameter<T> extends PostProcessEffectParameter<number> {
/**
* Create a new enum parameter.
* @param type - The type of the enum
* @param value - The default enum value of the parameter
*/
constructor(type, value: number) {
super(type, value, false);
}
}
25 changes: 15 additions & 10 deletions packages/core/src/postProcess/effects/BloomEffect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@ import { Color } from "@galacean/engine-math";
import { Shader, ShaderMacro, ShaderPass, ShaderProperty } from "../../shader";
import blitVs from "../../shaderlib/extra/Blit.vs.glsl";

import { Texture2D } from "../../texture";
import { PostProcessEffect } from "../PostProcessEffect";
import { PostProcessEffectParameter } from "../PostProcessEffectParameter";
import {
PostProcessEffectBoolParameter,
PostProcessEffectColorParameter,
PostProcessEffectEnumParameter,
PostProcessEffectFloatParameter,
PostProcessEffectTextureParameter
} from "../PostProcessEffectParameter";
import fragBlurH from "../shaders/Bloom/BloomBlurH.glsl";
import fragBlurV from "../shaders/Bloom/BloomBlurV.glsl";
import fragPrefilter from "../shaders/Bloom/BloomPrefilter.glsl";
Expand Down Expand Up @@ -55,43 +60,43 @@ export class BloomEffect extends PostProcessEffect {
* Controls whether to use bicubic sampling instead of bilinear sampling for the upSampling passes.
* @remarks This is slightly more expensive but helps getting smoother visuals.
*/
highQualityFiltering = new PostProcessEffectParameter(false);
highQualityFiltering = new PostProcessEffectBoolParameter(false);

/**
* Controls the starting resolution that this effect begins processing.
*/
downScale = new PostProcessEffectParameter(BloomDownScaleMode.Half);
downScale = new PostProcessEffectEnumParameter(BloomDownScaleMode, BloomDownScaleMode.Half);

/**
* Specifies a Texture to add smudges or dust to the bloom effect.
*/
dirtTexture = new PostProcessEffectParameter(<Texture2D>null);
dirtTexture = new PostProcessEffectTextureParameter(null);

/**
* Set the level of brightness to filter out pixels under this level.
* @remarks This value is expressed in gamma-space.
*/
threshold = new PostProcessEffectParameter(0.9, 0, Number.POSITIVE_INFINITY, true);
threshold = new PostProcessEffectFloatParameter(0.9, 0);

/**
* Controls the radius of the bloom effect.
*/
scatter = new PostProcessEffectParameter(0.7, 0, 1, true);
scatter = new PostProcessEffectFloatParameter(0.7, 0, 1);

/**
* Controls the strength of the bloom effect.
*/
intensity = new PostProcessEffectParameter(0, 0, Number.POSITIVE_INFINITY, true);
intensity = new PostProcessEffectFloatParameter(0, 0);

/**
* Controls the strength of the lens dirt.
*/
dirtIntensity = new PostProcessEffectParameter(0, 0, Number.POSITIVE_INFINITY, true);
dirtIntensity = new PostProcessEffectFloatParameter(0, 0);

/**
* Specifies the tint of the bloom effect.
*/
tint = new PostProcessEffectParameter(new Color(1, 1, 1, 1), true);
tint = new PostProcessEffectColorParameter(new Color(1, 1, 1, 1));

/** @inheritdoc */
override isValid(): boolean {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/postProcess/effects/TonemappingEffect.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ShaderMacro } from "../../shader";
import { PostProcessEffect } from "../PostProcessEffect";
import { PostProcessEffectParameter } from "../PostProcessEffectParameter";
import { PostProcessEffectEnumParameter } from "../PostProcessEffectParameter";

/**
* Options to select a tonemapping algorithm to use.
Expand Down Expand Up @@ -28,5 +28,5 @@ export class TonemappingEffect extends PostProcessEffect {
/**
* Use this to select a tonemapping algorithm to use.
*/
mode = new PostProcessEffectParameter(TonemappingMode.Neutral, false);
mode = new PostProcessEffectEnumParameter(TonemappingMode, TonemappingMode.Neutral);
}
Loading
Loading