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 asset destroy cache bug #622

Merged
merged 22 commits into from
Dec 30, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
3 changes: 1 addition & 2 deletions packages/core/src/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FeatureManager } from "./FeatureManager";
import { InputManager } from "./input/InputManager";
import { RenderQueueType } from "./material/enums/RenderQueueType";
import { Material } from "./material/Material";
import { ModelMesh, PrimitiveMesh } from "./mesh";
import { PhysicsManager } from "./physics";
import { IHardwareRenderer } from "./renderingHardwareInterface/IHardwareRenderer";
import { ClassPool } from "./RenderPipeline/ClassPool";
Expand Down Expand Up @@ -179,7 +178,7 @@ export class Engine extends EventDispatcher {
* @param physics - native physics Engine
*/
constructor(canvas: Canvas, hardwareRenderer: IHardwareRenderer, physics?: IPhysics, settings?: EngineSettings) {
super(null);
super();
this._hardwareRenderer = hardwareRenderer;
this._hardwareRenderer.init(canvas);
if (physics) {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/RenderPipeline/BasicRenderPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export class BasicRenderPipeline {
const program = _backgroundTextureMaterial.shader._getShaderProgram(engine, Shader._compileMacros);
program.bind();
program.uploadAll(program.materialUniformBlock, _backgroundTextureMaterial.shaderData);
program.uploadUngroupTextures();
program.uploadUnGroupTextures();

_backgroundTextureMaterial.renderState._apply(engine, false);
rhi.drawPrimitive(mesh, mesh.subMesh, program);
Expand Down Expand Up @@ -261,7 +261,7 @@ export class BasicRenderPipeline {
program.bind();
program.groupingOtherUniformBlock();
program.uploadAll(program.materialUniformBlock, shaderData);
program.uploadUngroupTextures();
program.uploadUnGroupTextures();

renderState._apply(engine, false);
rhi.drawPrimitive(mesh, mesh.subMesh, program);
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/RenderPipeline/RenderQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export class RenderQueue {
program.uploadAll(program.cameraUniformBlock, cameraData);
program.uploadAll(program.rendererUniformBlock, rendererData);
program.uploadAll(program.materialUniformBlock, materialData);
// Ungroup textures should upload default value, texture uint maybe change by logic of texture bind.
program.uploadUngroupTextures();
// UnGroup textures should upload default value, texture uint maybe change by logic of texture bind.
program.uploadUnGroupTextures();
program._uploadCamera = camera;
program._uploadRenderer = renderer;
program._uploadMaterial = material;
Expand All @@ -132,9 +132,9 @@ export class RenderQueue {
program.uploadTextures(program.materialUniformBlock, materialData);
}

// We only consider switchProgram case, because ungroup texture's value is always default.
// We only consider switchProgram case, because UnGroup texture's value is always default.
if (switchProgram) {
program.uploadUngroupTextures();
program.uploadUnGroupTextures();
}
}
material.renderState._apply(camera.engine, renderer.entity.transform._isFrontFaceInvert());
Expand Down
9 changes: 0 additions & 9 deletions packages/core/src/Scene.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export class Scene extends EngineObject {
/** @internal */
_globalShaderMacro: ShaderMacroCollection = new ShaderMacroCollection();

private _destroyed: boolean = false;
private _rootEntities: Entity[] = [];
private _ambientLight: AmbientLight;

Expand Down Expand Up @@ -72,13 +71,6 @@ export class Scene extends EngineObject {
return this._rootEntities;
}

/**
* Whether it's destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

/**
* Create scene.
* @param engine - Engine
Expand Down Expand Up @@ -218,7 +210,6 @@ export class Scene extends EngineObject {
this._activeCameras.length = 0;
(Scene.sceneFeatureManager as any)._objects = [];
this.shaderData._addRefCount(-1);
this._destroyed = true;
}

/**
Expand Down
14 changes: 2 additions & 12 deletions packages/core/src/asset/RefObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export abstract class RefObject extends EngineObject implements IRefObject {
isGCIgnored: boolean = false;

private _refCount: number = 0;
private _destroyed: boolean = false;

/**
* Counted by valid references.
Expand All @@ -19,13 +18,6 @@ export abstract class RefObject extends EngineObject implements IRefObject {
return this._refCount;
}

/**
* Whether it has been destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

protected constructor(engine: Engine) {
super(engine);
engine.resourceManager._addRefObject(this.instanceId, this);
Expand All @@ -43,7 +35,7 @@ export abstract class RefObject extends EngineObject implements IRefObject {
// resourceManager maybe null,because engine has destroyed.
// TODO:the right way to fix this is to ensure destroy all when call engine.destroy,thus don't need to add this project.
if (resourceManager) {
resourceManager._deleteAsset(this);
super.destroy();
resourceManager._deleteRefObject(this.instanceId);
}

Expand All @@ -53,7 +45,7 @@ export abstract class RefObject extends EngineObject implements IRefObject {
}
this._engine = null;
this._onDestroy();
this._destroyed = true;

return true;
}

Expand All @@ -66,15 +58,13 @@ export abstract class RefObject extends EngineObject implements IRefObject {

/**
* @internal
* Add reference resource.
*/
_addRefCount(value: number): void {
this._refCount += value;
}

/**
* @internal
* Remove reference resource.
*/
_addToResourceManager(path: string): void {
this._engine.resourceManager._addAsset(path, this);
Expand Down
17 changes: 8 additions & 9 deletions packages/core/src/asset/ResourceManager.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Engine, EngineObject } from "..";
import { ObjectValues } from "../base/Util";
import { AssetPromise } from "./AssetPromise";
import { Loader } from "./Loader";
import { LoadItem } from "./LoadItem";
import { RefObject } from "./RefObject";
import { Engine } from "..";
import { Loader } from "./Loader";
import { AssetType } from "./AssetType";
import { ObjectValues } from "../base/Util";

/**
* ResourceManager
Expand All @@ -17,10 +16,10 @@ export class ResourceManager {
/**
* @internal
*/
static _addLoader(type: string, loader: Loader<any>, extnames: string[]) {
static _addLoader(type: string, loader: Loader<any>, extNames: string[]) {
this._loaders[type] = loader;
for (let i = 0, len = extnames.length; i < len; i++) {
this._extTypeMapping[extnames[i]] = type;
for (let i = 0, len = extNames.length; i < len; i++) {
this._extTypeMapping[extNames[i]] = type;
}
}

Expand Down Expand Up @@ -145,15 +144,15 @@ export class ResourceManager {
/**
* @internal
*/
_addAsset(path: string, asset: RefObject): void {
_addAsset(path: string, asset: EngineObject): void {
this._assetPool[asset.instanceId] = path;
this._assetUrlPool[path] = asset;
}

/**
* @internal
*/
_deleteAsset(asset: RefObject): void {
_deleteAsset(asset: EngineObject): void {
const id = asset.instanceId;
const path = this._assetPool[id];
if (path) {
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/base/EngineObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,35 @@ export abstract class EngineObject {
@ignoreClone
protected _engine: Engine;

protected _destroyed: boolean = false;

/**
* Get the engine which the object belongs.
*/
get engine(): Engine {
return this._engine;
}

/**
* Whether it has been destroyed.
*/
get destroyed(): boolean {
return this._destroyed;
}

constructor(engine: Engine) {
this._engine = engine;
}

/**
* Destroy self.
*/
destroy(): void {
if (this._destroyed) return;

const resourceManager = this._engine.resourceManager;
resourceManager?._deleteAsset(this);
GuoLei1990 marked this conversation as resolved.
Show resolved Hide resolved

this._destroyed = true;
}
}
5 changes: 2 additions & 3 deletions packages/core/src/base/EventDispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { EngineObject } from "./EngineObject";
import { Event } from "./Event";
import { ignoreClone } from "../clone/CloneManager";
import { Event } from "./Event";

/**
* EventDispatcher, which can be inherited as a base class.
*/
export class EventDispatcher extends EngineObject {
export class EventDispatcher {
@ignoreClone
private _evts = Object.create(null);
private _evtCount = 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shader/ShaderProgram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export class ShaderProgram {
/**
* Upload ungroup texture shader data in shader uniform block.
*/
uploadUngroupTextures(): void {
uploadUnGroupTextures(): void {
const textureUniforms = this.otherUniformBlock.textureUniforms;
// textureUniforms property maybe null if ShaderUniformBlock not contain any texture.
if (textureUniforms) {
Expand Down
6 changes: 4 additions & 2 deletions packages/loader/src/scene-loader/Oasis.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EventDispatcher, ObjectValues } from "@oasis-engine/core";
import { Engine, EventDispatcher, ObjectValues } from "@oasis-engine/core";
import { AbilityManager } from "./AbilityManager";
import { NodeManager } from "./NodeManager";
import { SceneManager } from "./SceneManager";
Expand All @@ -15,9 +15,11 @@ export class Oasis extends EventDispatcher {
private schema: Schema;
public timeout: number;
private oasis = this;
public engine:Engine;

private constructor(private _options: Options, public readonly pluginManager: PluginManager) {
super(_options.engine);
super();
this.engine=_options.engine;
GuoLei1990 marked this conversation as resolved.
Show resolved Hide resolved
this.schema = _options.config;
this.timeout = _options.timeout;
_options.scripts = _options.scripts ?? {};
Expand Down