Skip to content

Commit 1e5dc5b

Browse files
finetjulfloryst
authored andcommitted
fix(vtkdataarray): fix double allocation when using vtkDataArray.resize
vtkDataArray.resize() actually changes the number of tuples, while vtkDataArray.allocate() simply allocate memory without changing the number of tuples. fix #3282
1 parent d19e4fe commit 1e5dc5b

File tree

7 files changed

+112
-6
lines changed

7 files changed

+112
-6
lines changed

Sources/Common/Core/CellArray/index.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ export interface vtkCellArray extends vtkDataArray {
3939

4040
/**
4141
* Insert a cell to this array in the next available slot.
42+
* This may re-allocate a new typed array if the current one is not large enough.
43+
* If the final size of the array is known up-front, it is more efficient to call
44+
* `allocate()` before calling `insertNextCell()` multiple times.
4245
* @param {Number[]} cellPointIds The list of point ids (NOT prefixed with the number of points)
4346
* @returns {Number} Idx of where the cell was inserted
4447
*/

Sources/Common/Core/CellArray/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ function vtkCellArray(publicAPI, model) {
6868

6969
/**
7070
* When `resize()` is being used, you then MUST use `insertNextCell()`.
71+
* @see vtkCellArray#insertNextCell
72+
* @see vtkDataArray#allocate
7173
*/
7274
publicAPI.resize = (requestedNumTuples) => {
7375
const oldNumTuples = publicAPI.getNumberOfTuples();

Sources/Common/Core/DataArray/index.d.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ export interface vtkDataArray extends vtkObject {
168168
*
169169
* @see insertNextTuple
170170
* @see getNumberOfTuples
171+
* @see allocate
171172
*
172173
* @param {Number} idx
173174
* @param {Array<Number>|TypedArray} tuple
@@ -189,6 +190,7 @@ export interface vtkDataArray extends vtkObject {
189190
* NOTE: May resize the data values array. "Safe" version of setTuple.
190191
*
191192
* @see insertTuple
193+
* @see allocate
192194
*
193195
* @param {Array<Number>|TypedArray} tuple
194196
* @returns {Number} Index of the inserted tuple.
@@ -303,6 +305,18 @@ export interface vtkDataArray extends vtkObject {
303305
t: float
304306
): void;
305307

308+
/**
309+
* Resize the array to the requested number of extra tuples
310+
* (added to the current number of tuples) and preserve data.
311+
* model.size WILL NOT be modified.
312+
* This is useful before multiple calls to `insertNextTuple()` or `insertNextTuples()`.
313+
* @param {Number} extraNumTuples Number of tuples to allocate memory wise.
314+
* @see insertNextTuple
315+
* @see insertNextTuples
316+
* @see allocate
317+
*/
318+
allocate(extraNumTuples: number): void;
319+
306320
/**
307321
* Resize the array to the requested number of tuples and preserve data.
308322
* Increasing the array size may allocate extra memory beyond what was
@@ -318,6 +332,7 @@ export interface vtkDataArray extends vtkObject {
318332
* @see insertNextTuple
319333
* @see insertNextTuples
320334
* @see initialize
335+
* @see allocate
321336
*/
322337
resize(requestedNumTuples: number): boolean;
323338

Sources/Common/Core/DataArray/index.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,21 +149,23 @@ function vtkDataArray(publicAPI, model) {
149149
}
150150

151151
const numComps = publicAPI.getNumberOfComponents();
152-
const curNumTuples = model.values.length / (numComps > 0 ? numComps : 1);
153-
if (requestedNumTuples === curNumTuples) {
152+
const numAllocatedTuples =
153+
model.values.length / (numComps > 0 ? numComps : 1);
154+
if (requestedNumTuples === numAllocatedTuples) {
154155
return true;
155156
}
156157

157-
if (requestedNumTuples > curNumTuples) {
158+
if (requestedNumTuples > numAllocatedTuples) {
158159
// Requested size is bigger than current size. Allocate enough
159160
// memory to fit the requested size and be more than double the
160161
// currently allocated memory.
161162
const oldValues = model.values;
162163
model.values = macro.newTypedArray(
163164
model.dataType,
164-
(requestedNumTuples + curNumTuples) * numComps
165+
(requestedNumTuples + numAllocatedTuples) * numComps
165166
);
166167
model.values.set(oldValues);
168+
// The actual number of tuples is not increased, only memory is allocated.
167169
return true;
168170
}
169171

@@ -181,6 +183,10 @@ function vtkDataArray(publicAPI, model) {
181183
publicAPI.modified();
182184
};
183185

186+
publicAPI.allocate = (extraNumTuples) => {
187+
resize(publicAPI.getNumberOfTuples() + extraNumTuples);
188+
};
189+
184190
publicAPI.resize = (requestedNumTuples) => {
185191
resize(requestedNumTuples);
186192
const newSize = requestedNumTuples * publicAPI.getNumberOfComponents();

Sources/Common/Core/DataArray/test/testDataArray.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,3 +428,83 @@ test('Test vtkDataArray findTuple', (t) => {
428428
t.equal(dataArray.findTuple(Float32Array.from([12, 13, 14])), 4);
429429
t.end();
430430
});
431+
432+
test('Test vtkDataArray allocate function', (t) => {
433+
// create an empty data array with 3 channel data.
434+
const da = vtkDataArray.newInstance({
435+
numberOfComponents: 3,
436+
empty: true,
437+
});
438+
439+
t.equal(da.getNumberOfTuples(), 0, 'empty');
440+
441+
da.allocate(2);
442+
let oldData = da.getData();
443+
444+
t.equal(
445+
da.getNumberOfTuples(),
446+
0,
447+
'allocate does not change number of tuples'
448+
);
449+
450+
da.insertNextTuple([1, 2, 3]);
451+
da.insertNextTuple([1, 2, 3]);
452+
453+
t.equal(da.getNumberOfTuples(), 2, 'inserted 2 tuples');
454+
t.equal(da.getData().buffer, oldData.buffer, 'no array allocation on insert');
455+
456+
da.allocate(2);
457+
458+
t.equal(
459+
da.getNumberOfTuples(),
460+
2,
461+
'allocate does not change number of tuples'
462+
);
463+
t.notEqual(
464+
da.getData().buffer,
465+
oldData.buffer,
466+
'reallocate array on allocate'
467+
);
468+
oldData = da.getData();
469+
470+
da.insertNextTuple([1, 2, 3]);
471+
da.insertNextTuple([1, 2, 3]);
472+
473+
t.ok(da.getNumberOfTuples() === 4, '2 more tuples');
474+
t.equal(da.getData().buffer, oldData.buffer, 'no array allocation on insert');
475+
476+
t.end();
477+
});
478+
479+
test('Test vtkDataArray resize function', (t) => {
480+
// create an empty data array with 3 channel data.
481+
const da = vtkDataArray.newInstance({
482+
numberOfComponents: 3,
483+
empty: true,
484+
});
485+
486+
t.ok(da.getNumberOfTuples() === 0, 'empty');
487+
488+
da.resize(2);
489+
490+
t.ok(da.getNumberOfTuples() === 2, 'resize does change the number of tuples');
491+
492+
da.insertNextTuple([1, 2, 3]);
493+
da.insertNextTuple([1, 2, 3]);
494+
495+
t.ok(da.getNumberOfTuples() === 4, 'inserted 2 tuples');
496+
497+
const oldData = da.getData();
498+
da.resize(2);
499+
500+
t.ok(da.getNumberOfTuples() === 2, 'resize reduces the number of tuples');
501+
t.equal(da.getData().buffer, oldData.buffer, 'no array allocation on shrink');
502+
503+
da.insertNextTuple([1, 2, 3]);
504+
da.insertNextTuple([1, 2, 3]);
505+
506+
t.ok(da.getNumberOfTuples() === 4, '2 more tuples');
507+
t.equal(da.getData().buffer, oldData.buffer, 'no array allocation on shrink');
508+
509+
t.end();
510+
});

Sources/IO/Geometry/DracoReader/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ function getPolyDataFromDracoGeometry(decoder, dracoGeometry) {
169169
const nCells = indices.length - 2;
170170

171171
const cells = vtkCellArray.newInstance();
172-
cells.resize((4 * indices.length) / 3);
172+
cells.allocate((4 * indices.length) / 3);
173173
for (let cellId = 0; cellId < nCells; cellId += 3) {
174174
const cell = indices.slice(cellId, cellId + 3);
175175
cells.insertNextCell(cell);

Sources/IO/Geometry/GLTFImporter/Reader.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ async function createPolyDataFromGLTFMesh(primitive) {
142142
vtkWarningMacro('GL_LINE_LOOP not implemented');
143143
break;
144144
default:
145-
cells.resize((4 * indices.length) / 3);
145+
cells.allocate((4 * indices.length) / 3);
146146
for (let cellId = 0; cellId < nCells; cellId += 3) {
147147
const cell = indices.slice(cellId, cellId + 3);
148148
cells.insertNextCell(cell);

0 commit comments

Comments
 (0)