Skip to content

Commit

Permalink
Minor improvements
Browse files Browse the repository at this point in the history
- Regenerate `vips.d.ts`.
- Move logic to our own `--js-library` implementation.
- Delete `fixed-threadpool-web.js` in favor of `vips-library.js`.
- Simplify added tests.
  • Loading branch information
kleisauke committed Jul 21, 2024
1 parent 1eaa6ab commit 7a6eb58
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 77 deletions.
4 changes: 2 additions & 2 deletions build/preamble_vips.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ declare module Vips {
isDeleted(): boolean;

/**
* Prevents the C++ object from being auto deleted
* @return `this`
* Prevents the C++ object from being auto deleted.
* @return `this`.
*/
preventAutoDelete(): T;
}
Expand Down
6 changes: 6 additions & 0 deletions lib/vips.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@
],
"ignore": [
"src/closure-externs/wasm-vips.js",
"src/fixed-threadpool-web.js",
"src/modules-pre.js",
"src/modules-post.js",
"src/vips-library.js",
"src/workaround-cors-pre.js"
],
"env": [
Expand Down
6 changes: 1 addition & 5 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ set(MAIN_LINK_OPTIONS
$<$<BOOL:${ENABLE_WASMFS}>:-sWASMFS>
$<$<BOOL:${ENABLE_MODULES}>:-sMAIN_MODULE=2>
$<$<BOOL:${ENABLE_MODULES}>:--pre-js=${CMAKE_CURRENT_SOURCE_DIR}/modules-pre.js>
--post-js=${CMAKE_CURRENT_SOURCE_DIR}/modules-post.js
--js-library=${CMAKE_CURRENT_SOURCE_DIR}/vips-library.js
-sAUTOLOAD_DYLIBS=0
-sABORTING_MALLOC=0
-sMALLOC=mimalloc
Expand Down Expand Up @@ -169,9 +169,6 @@ if ("web" IN_LIST ENVIRONMENT)
set(WEB_CORS_WORKAROUND
--pre-js=${CMAKE_CURRENT_SOURCE_DIR}/workaround-cors-pre.js
)
set(WEB_FIXED_THREADPOOL
--js-library=${CMAKE_CURRENT_SOURCE_DIR}/fixed-threadpool-web.js
)
set(WEB_MIN_TARGETS
-sMIN_FIREFOX_VERSION=89
-sMIN_CHROME_VERSION=91
Expand All @@ -183,7 +180,6 @@ if ("web" IN_LIST ENVIRONMENT)
set(WEB_LINK_OPTIONS
--use-preload-plugins
-sPTHREAD_POOL_SIZE='navigator.hardwareConcurrency+2+1'
${WEB_FIXED_THREADPOOL}
${WEB_MIN_TARGETS}
)

Expand Down
14 changes: 0 additions & 14 deletions src/fixed-threadpool-web.js

This file was deleted.

10 changes: 0 additions & 10 deletions src/modules-post.js

This file was deleted.

35 changes: 35 additions & 0 deletions src/vips-library.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
var LibraryVips = {
$VIPS__deps: [
#if ENVIRONMENT_MAY_BE_WEB
'$ENV',
#endif
'$ClassHandle',
'$deletionQueue',
],
$VIPS__postset: 'VIPS.init();',
$VIPS: {
init() {
#if ENVIRONMENT_MAY_BE_WEB
addOnPreRun(() => {
// Enforce a fixed thread pool by default on web
ENV['VIPS_MAX_THREADS'] = {{{ PTHREAD_POOL_SIZE }}};
});
#endif

// Add preventAutoDelete method to ClassHandle
Object.assign(ClassHandle.prototype, {
'preventAutoDelete'() {
const index = deletionQueue.indexOf(this);
if (index > -1) {
deletionQueue.splice(index, 1);
}
this.$$.deleteScheduled = false;
return this;
}
});
}
}
}

addToLibrary(LibraryVips);
DEFAULT_LIBRARY_FUNCS_TO_INCLUDE.push('$VIPS');
1 change: 1 addition & 0 deletions test/unit/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
</script>
<script src="lib/vips.js"></script>
<script type="module" src="test_arithmetic.js"></script>
<script type="module" src="test_auto_delete.js"></script>
<script type="module" src="test_block.js"></script>
<script type="module" src="test_colour.js"></script>
<script type="module" src="test_connection.js"></script>
Expand Down
79 changes: 35 additions & 44 deletions test/unit/test_auto_delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,62 +11,53 @@ describe('auto delete', () => {
const im = vips.Image.black(100, 100);
expect(vips.deletionQueue.length).to.equal(1);
expect(() => im.clone()).to.throw(/Object already scheduled for deletion/);

im.gaussblur(0.3); // creates new handle
expect(vips.deletionQueue.length).to.equal(2);

cleanup();
expect(vips.deletionQueue.length).to.equal(0);
});

it('preventAutoDelete', function () {
const im = new vips.Image();
const source = new vips.SourceCustom();
expect(vips.deletionQueue.length).to.equal(2);
im.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(1);
source.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(0);
describe('preventAutoDelete', () => {
it('all handles', function () {
const handles = Object.entries(vips).filter(
([key, Handle]) =>
key !== 'Object' && !!Handle?.prototype?.preventAutoDelete
);
expect(handles.length).to.equal(12);

// cloned objects do not retain prevent auto delete status
const cloned = im.clone();
expect(vips.deletionQueue.length).to.equal(1);
cleanup();
expect(vips.deletionQueue.length).to.equal(0);
for (const [name] of handles) {
const h = new vips[name]();
expect(vips.deletionQueue.length).to.equal(1);

im.delete(); // should not fail
source.delete();
expect(() => im.delete()).to.throw(/Image instance already deleted/);
expect(() => source.delete()).to.throw(/SourceCustom instance already deleted/);
// already deleted by cleanup, should fail
expect(() => cloned.delete()).to.throw(/Image instance already deleted/);
});
h.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(0);

h.delete();
expect(() => h.delete()).to.throw(new RegExp(`${name} instance already deleted`));
}
});

it('preventAutoDelete on all handles', function () {
const handles = Object.entries(vips).filter(
([key, Handle]) =>
key !== 'Object' && !!Handle?.prototype?.preventAutoDelete
);
expect(handles.length).to.equal(12);
it('cloned handle', function () {
const im = new vips.Image();
expect(vips.deletionQueue.length).to.equal(1);

im.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(0);

for (const [name] of handles) {
const h = new vips[name]();
// cloned objects do not retain prevent auto delete status
const cloned = im.clone();
expect(vips.deletionQueue.length).to.equal(1);
h.preventAutoDelete();

cleanup();
expect(vips.deletionQueue.length).to.equal(0);
h.delete();
expect(() => h.delete()).to.throw(new RegExp(`${name} instance already deleted`));
}
});

it('preventAutoDelete on cloned handle', function () {
const im = new vips.Image();
expect(vips.deletionQueue.length).to.equal(1);
im.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(0);
const cloned = im.clone();
expect(vips.deletionQueue.length).to.equal(1);
cloned.preventAutoDelete();
expect(vips.deletionQueue.length).to.equal(0);
cloned.delete();
im.delete();
im.delete(); // should not fail
expect(() => im.delete()).to.throw(/Image instance already deleted/);

// already deleted by cleanup, should fail
expect(() => cloned.delete()).to.throw(/Image instance already deleted/);
});
});
});

0 comments on commit 7a6eb58

Please sign in to comment.