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 clear flag of internal render target #2444

Merged
merged 19 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
29 changes: 19 additions & 10 deletions e2e/case/.mockForE2E.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Camera, Engine, RenderTarget, Texture2D } from "@galacean/engine-core";
import { Camera, Engine, RenderTarget, Texture2D, TextureFormat } from "@galacean/engine-core";

export const updateForE2E = (engine, deltaTime = 100, loopTime = 10) => {
engine._vSyncCount = Infinity;
Expand All @@ -19,7 +19,7 @@ let flipYCanvas: HTMLCanvasElement = null;

export function initScreenshot(
engine: Engine,
camera: Camera,
camera: Camera | Camera[],
width: number = 1200,
height: number = 800,
flipY = false,
Expand All @@ -38,15 +38,25 @@ export function initScreenshot(
const isPaused = engine.isPaused;
engine.pause();

const originalTarget = camera.renderTarget;
const cameras = Array.isArray(camera) ? camera : [camera];
const callbacks = [];
const renderColorTexture = new Texture2D(engine, width, height);
const renderTargetData = new Uint8Array(width * height * 4);
const renderTarget = new RenderTarget(engine, width, height, renderColorTexture, undefined, 1);
const renderTarget = new RenderTarget(engine, width, height, renderColorTexture, TextureFormat.Depth24Stencil8, 1);

// render to off-screen
camera.renderTarget = renderTarget;
camera.aspectRatio = width / height;
camera.render();
cameras.forEach((camera) => {
const originalTarget = camera.renderTarget;

// render to off-screen
camera.renderTarget = renderTarget;
camera.aspectRatio = width / height;
camera.render();

callbacks.push(() => {
camera.renderTarget = originalTarget;
camera.resetAspectRatio();
});
});

renderColorTexture.getPixelBuffer(0, 0, width, height, 0, renderTargetData);

Expand Down Expand Up @@ -94,8 +104,7 @@ export function initScreenshot(
// window.URL.revokeObjectURL(url);

// revert
camera.renderTarget = originalTarget;
camera.resetAspectRatio();
callbacks.forEach((cb) => cb());
!isPaused && engine.resume();
},
isPNG ? "image/png" : "image/jpeg",
Expand Down
96 changes: 96 additions & 0 deletions e2e/case/multi-camera-no-clear.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/**
* @title Multi camera no clear
* @category Advance
*/
import {
BlinnPhongMaterial,
Camera,
CameraClearFlags,
Color,
Engine,
Layer,
Logger,
MeshRenderer,
PrimitiveMesh,
Scene,
WebGLEngine,
WebGLMode
} from "@galacean/engine";
import { initScreenshot, updateForE2E } from "./.mockForE2E";

Logger.enable();
WebGLEngine.create({
canvas: "canvas",
graphicDeviceOptions: {
webGLMode: WebGLMode.WebGL2
}
}).then((engine) => {
engine.canvas.resizeByClientSize();

initFirstScene(engine);
engine.run();
});

function initFirstScene(engine: Engine): Scene {
const scene = engine.sceneManager.scenes[0];
const rootEntity = scene.createRootEntity();

// const renderColorTexture = new Texture2D(engine, 1024, 1024);
// const depthTexture = new Texture2D(engine, 1024, 1024, TextureFormat.Depth24Stencil8, false);
// const renderTarget = new RenderTarget(engine, 1024, 1024, renderColorTexture, TextureFormat.Depth);
// const renderTarget = new RenderTarget(engine, 1024, 1024, renderColorTexture, depthTexture);

// Create full screen camera
const cameraEntity = rootEntity.createChild("fullscreen-camera");
const camera = cameraEntity.addComponent(Camera);
// camera.renderTarget = renderTarget;
camera.cullingMask = Layer.Layer0;
cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.transform.setPosition(-3, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createSphere(engine, 2, 24);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
renderer.setMaterial(material);

{
const cameraEntity = rootEntity.createChild("window-camera");
const camera2 = cameraEntity.addComponent(Camera);
// camera2.renderTarget = renderTarget;
camera2.cullingMask = Layer.Layer1;
camera2.enablePostProcess = true;
camera2.enableHDR = true;
camera2.clearFlags = CameraClearFlags.None;
camera2.msaaSamples = 4;

// @ts-ignore
const bloomEffect = scene._postProcessManager._bloomEffect;
// @ts-ignore
const tonemappingEffect = scene._postProcessManager._tonemappingEffect;
GuoLei1990 marked this conversation as resolved.
Show resolved Hide resolved

bloomEffect.enabled = true;
tonemappingEffect.enabled = true;
bloomEffect.threshold = 0.1;
bloomEffect.intensity = 2;
cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.layer = Layer.Layer1;
cubeEntity.transform.setPosition(-2, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createSphere(engine, 2, 24);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
material.emissiveColor.set(1, 0, 0, 1);
renderer.setMaterial(material);

updateForE2E(engine);
initScreenshot(engine, [camera, camera2]);
}

return scene;
}
85 changes: 85 additions & 0 deletions e2e/case/multi-scene-clear.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* @title Multi scene clear
* @category Advance
*/
import {
BlinnPhongMaterial,
Camera,
CameraClearFlags,
Color,
Engine,
Logger,
MeshRenderer,
PrimitiveMesh,
Scene,
WebGLEngine
} from "@galacean/engine";
import { initScreenshot, updateForE2E } from "./.mockForE2E";

Logger.enable();
WebGLEngine.create({ canvas: "canvas" }).then((engine) => {
engine.canvas.resizeByClientSize();

const firstCamera = initFirstScene(engine);
const secondCamera = initSecondScene(engine);

updateForE2E(engine);
initScreenshot(engine, [firstCamera, secondCamera]);
// initScreenshot(engine, [secondCamera, firstCamera]);
});

function initFirstScene(engine: Engine): Camera {
const scene = engine.sceneManager.scenes[0];
const rootEntity = scene.createRootEntity();

// Create full screen camera
const cameraEntity = rootEntity.createChild("fullscreen-camera");
const camera = cameraEntity.addComponent(Camera);
cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.transform.setPosition(-2, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createCuboid(engine, 2, 2, 2);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
renderer.setMaterial(material);

return camera;
}

function initSecondScene(engine: Engine): Camera {
// Init window camera
const scene = new Scene(engine);
engine.sceneManager.addScene(scene);
const rootEntity = scene.createRootEntity();

const cameraEntity = rootEntity.createChild("window-camera");
const camera = cameraEntity.addComponent(Camera);
camera.enablePostProcess = true;
// camera.clearFlags = CameraClearFlags.None;

// @ts-ignore
const bloomEffect = scene._postProcessManager._bloomEffect;
// @ts-ignore
const tonemappingEffect = scene._postProcessManager._tonemappingEffect;
Comment on lines +63 to +66
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid direct access to internal post-process manager properties.

Using @ts-ignore to access internal properties is risky and could break with future engine updates. Consider adding proper public APIs for accessing these effects if they're needed for testing.

Consider one of these approaches:

  1. Add public getters for these effects in the PostProcessManager
  2. Create a test utility function that safely accesses these properties
  3. Use public APIs to configure post-processing if available


bloomEffect.enabled = true;
tonemappingEffect.enabled = true;
bloomEffect.threshold = 0.1;
bloomEffect.intensity = 2;
Comment on lines +68 to +71
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for post-processing effects.

The code assumes the effects exist and are enabled successfully. Consider adding checks to validate the effects are available and properly initialized.

+if (!bloomEffect || !tonemappingEffect) {
+  throw new Error("Required post-processing effects are not available");
+}
 bloomEffect.enabled = true;
 tonemappingEffect.enabled = true;
 bloomEffect.threshold = 0.1;
 bloomEffect.intensity = 2;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bloomEffect.enabled = true;
tonemappingEffect.enabled = true;
bloomEffect.threshold = 0.1;
bloomEffect.intensity = 2;
if (!bloomEffect || !tonemappingEffect) {
throw new Error("Required post-processing effects are not available");
}
bloomEffect.enabled = true;
tonemappingEffect.enabled = true;
bloomEffect.threshold = 0.1;
bloomEffect.intensity = 2;

cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.transform.setPosition(2, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createCuboid(engine, 2, 2, 2);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
material.emissiveColor.set(1, 0, 0, 1);
renderer.setMaterial(material);

return camera;
}
91 changes: 91 additions & 0 deletions e2e/case/multi-scene-no-clear.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* @title Multi scene no clear
* @category Advance
*/
import {
BlinnPhongMaterial,
Camera,
CameraClearFlags,
Color,
Engine,
Logger,
MeshRenderer,
PrimitiveMesh,
Scene,
WebGLEngine,
WebGLMode
} from "@galacean/engine";
import { initScreenshot, updateForE2E } from "./.mockForE2E";

Logger.enable();
WebGLEngine.create({
canvas: "canvas",
graphicDeviceOptions: {
webGLMode: WebGLMode.WebGL2
}
}).then((engine) => {
engine.canvas.resizeByClientSize();

const firstCamera = initFirstScene(engine);
const secondCamera = initSecondScene(engine);

updateForE2E(engine);
initScreenshot(engine, [firstCamera, secondCamera]);
// initScreenshot(engine, [secondCamera, firstCamera]);
});

function initFirstScene(engine: Engine): Camera {
const scene = engine.sceneManager.scenes[0];
const rootEntity = scene.createRootEntity();

// Create full screen camera
const cameraEntity = rootEntity.createChild("fullscreen-camera");
const camera = cameraEntity.addComponent(Camera);
cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.transform.setPosition(-2, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createCuboid(engine, 2, 2, 2);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
renderer.setMaterial(material);

return camera;
}

function initSecondScene(engine: Engine): Camera {
// Init window camera
const scene = new Scene(engine);
engine.sceneManager.addScene(scene);
const rootEntity = scene.createRootEntity();

const cameraEntity = rootEntity.createChild("window-camera");
const camera = cameraEntity.addComponent(Camera);
camera.enablePostProcess = true;
camera.clearFlags = CameraClearFlags.None;

// @ts-ignore
const bloomEffect = scene._postProcessManager._bloomEffect;
// @ts-ignore
const tonemappingEffect = scene._postProcessManager._tonemappingEffect;
Comment on lines +69 to +72
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid accessing internal properties with @ts-ignore.

Direct access to internal properties (_postProcessManager, _bloomEffect, _tonemappingEffect) with @ts-ignore is fragile and could break with engine updates.

Consider:

  1. Using public APIs if available
  2. If not available, propose adding public methods to access these effects
  3. Document why internal access is necessary if it can't be avoided


bloomEffect.enabled = true;
tonemappingEffect.enabled = true;
bloomEffect.threshold = 0.1;
bloomEffect.intensity = 2;
cameraEntity.transform.setPosition(0, 0, 20);

// Create cube
const cubeEntity = rootEntity.createChild("cube");
cubeEntity.transform.setPosition(2, 0, 3);
const renderer = cubeEntity.addComponent(MeshRenderer);
renderer.mesh = PrimitiveMesh.createCuboid(engine, 2, 2, 2);
const material = new BlinnPhongMaterial(engine);
material.baseColor = new Color(1, 0, 0, 1);
material.emissiveColor.set(1, 0, 0, 1);
renderer.setMaterial(material);

return camera;
}
15 changes: 15 additions & 0 deletions e2e/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,21 @@ export const E2E_CONFIG = {
category: "Advance",
caseFileName: "project-loader",
threshold: 0.4
},
MultiSceneClear: {
category: "Advance",
caseFileName: "multi-scene-clear",
threshold: 0.2
},
MultiSceneNoClear: {
category: "Advance",
caseFileName: "multi-scene-no-clear",
threshold: 0.2
},
MultiCameraNoClear: {
category: "Advance",
caseFileName: "multi-camera-no-clear",
threshold: 0.2
}
}
};
3 changes: 3 additions & 0 deletions e2e/fixtures/originImage/Advance_multi-camera-no-clear.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions e2e/fixtures/originImage/Advance_multi-scene-clear.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions e2e/fixtures/originImage/Advance_multi-scene-no-clear.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 8 additions & 4 deletions packages/core/src/Camera.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class Camera extends Component {
/**
* Multi-sample anti-aliasing samples when use independent canvas mode.
*
* @remarks The `independentCanvasEnabled` property should be `true` to take effect, otherwise it will be invalid.
* @remarks It will take effect when `independentCanvasEnabled` property is `true`, otherwise it will be invalid.
*/
msaaSamples: MSAASamples = MSAASamples.None;

Expand Down Expand Up @@ -162,15 +162,19 @@ export class Camera extends Component {

/**
* Whether independent canvas is enabled.
*
* @remarks If true, the msaa in viewport can turn or off independently by `msaaSamples` property.
*/
get independentCanvasEnabled(): boolean {
if (this.enableHDR || (this.enablePostProcess && this.scene._postProcessManager.hasActiveEffect)) {
// Uber pass need internal RT
if (this.enablePostProcess && this.scene._postProcessManager.hasActiveEffect) {
return true;
}

return this.opaqueTextureEnabled && !this._renderTarget;
if (this.enableHDR || this.opaqueTextureEnabled) {
return this._getInternalColorTextureFormat() !== this.renderTarget?.getColorTexture(0).format;
}

return false;
}

/**
Expand Down
Loading
Loading