Skip to content

Commit

Permalink
fix(core): premature release of texture prop (#7860)
Browse files Browse the repository at this point in the history
  • Loading branch information
Pessimistress committed Apr 27, 2023
1 parent 7f2dd44 commit 6f57cd3
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 9 deletions.
6 changes: 3 additions & 3 deletions modules/core/src/lifecycle/prop-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,13 @@ const TYPE_DEFINITIONS = {
if (!context || !context.gl) {
return null;
}
return createTexture(context.gl, value, {
return createTexture(component.id, context.gl, value, {
...propType.parameters,
...component.props.textureParameters
});
},
release: value => {
destroyTexture(value);
release: (value, propType: ImagePropType, component) => {
destroyTexture(component.id, value);
}
}
} as const;
Expand Down
10 changes: 6 additions & 4 deletions modules/core/src/utils/texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ const DEFAULT_TEXTURE_PARAMETERS: Record<number, number> = {
};

// Track the textures that are created by us. They need to be released when they are no longer used.
const internalTextures: Record<string, boolean> = {};
const internalTextures: Record<string, string> = {};

export function createTexture(
owner: string,
gl: WebGLRenderingContext,
image: any,
parameters: Record<number, number>
Expand Down Expand Up @@ -43,15 +44,16 @@ export function createTexture(
}
});
// Track this texture
internalTextures[texture.id] = true;
internalTextures[texture.id] = owner;
return texture;
}

export function destroyTexture(texture: Texture2D) {
export function destroyTexture(owner: string, texture: Texture2D) {
if (!texture || !(texture instanceof Texture2D)) {
return;
}
if (internalTextures[texture.id]) {
// Only delete the texture if requested by the same layer that created it
if (internalTextures[texture.id] === owner) {
texture.delete();
delete internalTextures[texture.id];
}
Expand Down
3 changes: 1 addition & 2 deletions modules/extensions/src/fill-style/fill-style-extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ export default class FillStyleExtension extends LayerExtension<FillStyleExtensio
}

finalizeState(this: Layer<FillStyleExtensionProps>) {
const {patternTexture, emptyTexture} = this.state;
patternTexture?.delete();
const {emptyTexture} = this.state;
emptyTexture?.delete();
}

Expand Down
9 changes: 9 additions & 0 deletions test/modules/extensions/fill-style.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ test('FillStyleExtension#PolygonLayer', t => {
t.notOk(strokeLayer.state.emptyTexture, 'should not be enabled in PathLayer');
t.notOk('fill_patternMask' in uniforms, 'should not be enabled in PathLayer');
}
},
{
title: `Finalizing a sublayer should not affect the parent layer's loaded props`,
updateProps: {
data: []
},
onAfterUpdate: ({layer}) => {
t.ok(layer.props.fillPatternAtlas.handle, 'fillPatternAtlas texture is not deleted');
}
}
];

Expand Down

0 comments on commit 6f57cd3

Please sign in to comment.