Skip to content

Commit dca1dc2

Browse files
PaulHaxsankhesh
authored andcommitted
fix(Actor): clean up cull state in lifecycle hooks
vtkOpenGLPolyDataMapper2D enables CULL_FACE on entry but never restores it. In multi-child-render-window scenarios that share one WebGL context, the leaked cull state carries into the next renderer's clear() and culls the back-facing image quad, producing a black render. The same latent leak exists for any 3D actor with vtkProperty.BackfaceCulling or FrontfaceCulling enabled when followed by an ImageMapper. Move cull-state cleanup into the Actor and Actor2D postpass hooks (prepass=false), mirroring the C++ vtkOpenGLProperty::PostRender idiom. Setup of cull state stays in the mappers; teardown runs unconditionally on actor exit. The cache from the previous commit makes the call free when cull is already off. Adds testCullFaceLeak with two assertions: the original Actor2D-into- image-window case, and a 3D actor with backfaceCulling=true to cover the broader leak class.
1 parent bb40d26 commit dca1dc2

3 files changed

Lines changed: 252 additions & 17 deletions

File tree

Sources/Rendering/OpenGL/Actor/index.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,16 @@ function vtkOpenGLActor(publicAPI, model) {
142142
if (prepass) {
143143
model.context.depthMask(true);
144144
publicAPI.activateTextures();
145-
} else if (model.activeTextures) {
146-
for (let index = 0; index < model.activeTextures.length; index++) {
147-
model.activeTextures[index].deactivate();
145+
} else {
146+
if (model.activeTextures) {
147+
for (let index = 0; index < model.activeTextures.length; index++) {
148+
model.activeTextures[index].deactivate();
149+
}
148150
}
151+
// Restore baseline cull state so a mapper that enabled culling
152+
// cannot leak into later actors or the next renderer's clear().
153+
// Mirrors C++ vtkOpenGLProperty::PostRender.
154+
model._openGLRenderWindow.disableCullFace();
149155
}
150156
};
151157

@@ -157,10 +163,13 @@ function vtkOpenGLActor(publicAPI, model) {
157163
model.renderable.getNestedPickable()
158164
);
159165
publicAPI.activateTextures();
160-
} else if (model.activeTextures) {
161-
for (let index = 0; index < model.activeTextures.length; index++) {
162-
model.activeTextures[index].deactivate();
166+
} else {
167+
if (model.activeTextures) {
168+
for (let index = 0; index < model.activeTextures.length; index++) {
169+
model.activeTextures[index].deactivate();
170+
}
163171
}
172+
model._openGLRenderWindow.disableCullFace();
164173
}
165174
};
166175

Sources/Rendering/OpenGL/Actor2D/index.js

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,16 @@ function vtkOpenGLActor2D(publicAPI, model) {
129129
if (prepass) {
130130
model.context.depthMask(true);
131131
publicAPI.activateTextures();
132-
} else if (model.activeTextures) {
133-
// deactivate textures
134-
for (let index = 0; index < model.activeTextures.length; index++) {
135-
model.activeTextures[index].deactivate();
132+
} else {
133+
if (model.activeTextures) {
134+
for (let index = 0; index < model.activeTextures.length; index++) {
135+
model.activeTextures[index].deactivate();
136+
}
136137
}
138+
// Restore baseline cull state so a mapper that enabled culling
139+
// cannot leak into later actors or the next renderer's clear().
140+
// Mirrors C++ vtkOpenGLProperty::PostRender.
141+
model._openGLRenderWindow.disableCullFace();
137142
}
138143
};
139144

@@ -142,10 +147,13 @@ function vtkOpenGLActor2D(publicAPI, model) {
142147
if (prepass) {
143148
model.context.depthMask(false);
144149
publicAPI.activateTextures();
145-
} else if (model.activeTextures) {
146-
for (let index = 0; index < model.activeTextures.length; index++) {
147-
model.activeTextures[index].deactivate();
150+
} else {
151+
if (model.activeTextures) {
152+
for (let index = 0; index < model.activeTextures.length; index++) {
153+
model.activeTextures[index].deactivate();
154+
}
148155
}
156+
model._openGLRenderWindow.disableCullFace();
149157
}
150158
};
151159

@@ -154,11 +162,13 @@ function vtkOpenGLActor2D(publicAPI, model) {
154162
if (prepass) {
155163
model.context.depthMask(true);
156164
publicAPI.activateTextures();
157-
} else if (model.activeTextures) {
158-
// deactivate textures
159-
for (let index = 0; index < model.activeTextures.length; index++) {
160-
model.activeTextures[index].deactivate();
165+
} else {
166+
if (model.activeTextures) {
167+
for (let index = 0; index < model.activeTextures.length; index++) {
168+
model.activeTextures[index].deactivate();
169+
}
161170
}
171+
model._openGLRenderWindow.disableCullFace();
162172
}
163173
};
164174
}
Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
import test from 'tape';
2+
import testUtils from 'vtk.js/Sources/Testing/testUtils';
3+
4+
import vtkDataArray from 'vtk.js/Sources/Common/Core/DataArray';
5+
import vtkImageData from 'vtk.js/Sources/Common/DataModel/ImageData';
6+
import vtkLineSource from 'vtk.js/Sources/Filters/Sources/LineSource';
7+
import vtkSphereSource from 'vtk.js/Sources/Filters/Sources/SphereSource';
8+
import vtkActor from 'vtk.js/Sources/Rendering/Core/Actor';
9+
import vtkActor2D from 'vtk.js/Sources/Rendering/Core/Actor2D';
10+
import vtkCoordinate from 'vtk.js/Sources/Rendering/Core/Coordinate';
11+
import vtkImageMapper from 'vtk.js/Sources/Rendering/Core/ImageMapper';
12+
import vtkImageSlice from 'vtk.js/Sources/Rendering/Core/ImageSlice';
13+
import vtkMapper from 'vtk.js/Sources/Rendering/Core/Mapper';
14+
import vtkMapper2D from 'vtk.js/Sources/Rendering/Core/Mapper2D';
15+
import vtkRenderer from 'vtk.js/Sources/Rendering/Core/Renderer';
16+
import vtkRenderWindow from 'vtk.js/Sources/Rendering/Core/RenderWindow';
17+
import 'vtk.js/Sources/Rendering/Misc/RenderingAPIs';
18+
19+
import { SlicingMode } from 'vtk.js/Sources/Rendering/Core/ImageMapper/Constants';
20+
21+
function createWhiteImage() {
22+
const image = vtkImageData.newInstance();
23+
image.setDimensions(16, 16, 1);
24+
image.getPointData().setScalars(
25+
vtkDataArray.newInstance({
26+
numberOfComponents: 1,
27+
values: new Uint8Array(16 * 16).fill(255),
28+
})
29+
);
30+
return image;
31+
}
32+
33+
function createImageSlice(image) {
34+
const mapper = vtkImageMapper.newInstance();
35+
mapper.setInputData(image);
36+
mapper.setSlicingMode(SlicingMode.K);
37+
mapper.setSlice(0);
38+
39+
const actor = vtkImageSlice.newInstance();
40+
actor.setMapper(mapper);
41+
actor.getProperty().setColorWindow(255);
42+
actor.getProperty().setColorLevel(127);
43+
44+
return { actor, mapper };
45+
}
46+
47+
// A leaking case enables CULL_FACE without restoring it. It returns the
48+
// resources to register for cleanup and a function to add it to a renderer.
49+
function makeLineActor2DCase() {
50+
const line = vtkLineSource.newInstance({
51+
point1: [2, 2, 0],
52+
point2: [14, 14, 0],
53+
});
54+
55+
const coordinate = vtkCoordinate.newInstance();
56+
coordinate.setCoordinateSystemToWorld();
57+
58+
const mapper = vtkMapper2D.newInstance();
59+
mapper.setInputConnection(line.getOutputPort());
60+
mapper.setTransformCoordinate(coordinate);
61+
mapper.setScalarVisibility(false);
62+
63+
const actor = vtkActor2D.newInstance();
64+
actor.setMapper(mapper);
65+
66+
return {
67+
resources: [actor, mapper, line, coordinate],
68+
addToRenderer: (renderer) => renderer.addActor2D(actor),
69+
};
70+
}
71+
72+
function makeBackfaceCulledSphereCase() {
73+
const source = vtkSphereSource.newInstance({
74+
center: [7.5, 7.5, 0],
75+
radius: 1,
76+
});
77+
78+
const mapper = vtkMapper.newInstance();
79+
mapper.setInputConnection(source.getOutputPort());
80+
81+
const actor = vtkActor.newInstance();
82+
actor.setMapper(mapper);
83+
actor.getProperty().setBackfaceCulling(true);
84+
85+
return {
86+
resources: [actor, mapper, source],
87+
addToRenderer: (renderer) => renderer.addActor(actor),
88+
};
89+
}
90+
91+
function setCameraToBackFace(renderer) {
92+
const camera = renderer.getActiveCamera();
93+
camera.setParallelProjection(true);
94+
camera.setPosition(7.5, 7.5, -50);
95+
camera.setFocalPoint(7.5, 7.5, 0);
96+
camera.setViewUp(0, 1, 0);
97+
camera.setParallelScale(9);
98+
renderer.resetCameraClippingRange();
99+
}
100+
101+
function addChildRenderWindow(
102+
parentRenderWindow,
103+
parentView,
104+
container,
105+
image
106+
) {
107+
const renderWindow = vtkRenderWindow.newInstance();
108+
parentRenderWindow.addRenderWindow(renderWindow);
109+
110+
const view = parentView.addMissingNode(renderWindow);
111+
renderWindow.addView(view);
112+
view.setContainer(container);
113+
view.setSize(64, 64);
114+
view.getCanvas().style.width = '64px';
115+
view.getCanvas().style.height = '64px';
116+
117+
const renderer = vtkRenderer.newInstance();
118+
renderer.setBackground(0, 0, 0);
119+
renderWindow.addRenderer(renderer);
120+
121+
const imageSlice = createImageSlice(image);
122+
renderer.addActor(imageSlice.actor);
123+
setCameraToBackFace(renderer);
124+
125+
return { renderWindow, renderer, view, imageSlice };
126+
}
127+
128+
function getCenterPixel(canvas) {
129+
const context = canvas.getContext('2d');
130+
const { data } = context.getImageData(
131+
Math.floor(canvas.width / 2),
132+
Math.floor(canvas.height / 2),
133+
1,
134+
1
135+
);
136+
return Array.from(data);
137+
}
138+
139+
function isBright(pixel) {
140+
return pixel[0] > 128 && pixel[1] > 128 && pixel[2] > 128 && pixel[3] > 0;
141+
}
142+
143+
// child1 shows an image only. child2 shows an image plus a leaking actor.
144+
// Both children share one WebGL context. If the leaking actor's cull state
145+
// is not cleaned up, child1's back-facing image quad is culled on re-render.
146+
function runLeakTest(t, leakingCase) {
147+
const gc = testUtils.createGarbageCollector(t);
148+
const root = document.querySelector('body');
149+
const childContainer1 = gc.registerDOMElement(document.createElement('div'));
150+
const childContainer2 = gc.registerDOMElement(document.createElement('div'));
151+
root.appendChild(childContainer1);
152+
root.appendChild(childContainer2);
153+
154+
const parentRenderWindow = gc.registerResource(vtkRenderWindow.newInstance());
155+
const parentView = gc.registerResource(
156+
parentRenderWindow.newAPISpecificView('WebGL')
157+
);
158+
parentRenderWindow.addView(parentView);
159+
parentView.initialize();
160+
161+
const image = gc.registerResource(createWhiteImage());
162+
const child1 = addChildRenderWindow(
163+
parentRenderWindow,
164+
parentView,
165+
childContainer1,
166+
image
167+
);
168+
const child2 = addChildRenderWindow(
169+
parentRenderWindow,
170+
parentView,
171+
childContainer2,
172+
image
173+
);
174+
175+
leakingCase.addToRenderer(child2.renderer);
176+
177+
[
178+
child1.renderWindow,
179+
child1.renderer,
180+
child1.imageSlice.actor,
181+
child1.imageSlice.mapper,
182+
child2.renderWindow,
183+
child2.renderer,
184+
child2.imageSlice.actor,
185+
child2.imageSlice.mapper,
186+
...leakingCase.resources,
187+
].forEach((resource) => gc.registerResource(resource));
188+
189+
parentView.resizeFromChildRenderWindows();
190+
191+
parentRenderWindow.render();
192+
const firstRenderPixel = getCenterPixel(child1.view.getCanvas());
193+
t.ok(
194+
isBright(firstRenderPixel),
195+
`first child slice is visible on initial render (${firstRenderPixel})`
196+
);
197+
198+
parentRenderWindow.render();
199+
const secondRenderPixel = getCenterPixel(child1.view.getCanvas());
200+
t.ok(
201+
isBright(secondRenderPixel),
202+
`first child slice remains visible after leaking actor render (${secondRenderPixel})`
203+
);
204+
205+
gc.releaseResources();
206+
}
207+
208+
test.onlyIfWebGL(
209+
'Test PolyDataMapper2D cull face does not leak between child render windows',
210+
(t) => runLeakTest(t, makeLineActor2DCase())
211+
);
212+
213+
test.onlyIfWebGL(
214+
'Test 3D actor backface culling does not leak between child render windows',
215+
(t) => runLeakTest(t, makeBackfaceCulledSphereCase())
216+
);

0 commit comments

Comments
 (0)