From b3da5370c06e5e0c7755ba633ae0e1bcef7ca640 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:08:09 -0700 Subject: [PATCH 01/13] Add a new method to get the representation of an image --- atom/common/api/atom_api_native_image.h | 1 + 1 file changed, 1 insertion(+) diff --git a/atom/common/api/atom_api_native_image.h b/atom/common/api/atom_api_native_image.h index 1f0fe946ba51..8bc213040415 100644 --- a/atom/common/api/atom_api_native_image.h +++ b/atom/common/api/atom_api_native_image.h @@ -61,6 +61,7 @@ class NativeImage : public mate::Wrappable { private: v8::Local ToPNG(v8::Isolate* isolate); v8::Local ToJPEG(v8::Isolate* isolate, int quality); + v8::Local AsNativeRepresentation(v8::Isolate* isolate, mate::Arguments* args); std::string ToDataURL(); bool IsEmpty(); gfx::Size GetSize(); From 262abc43f80811747c8689b85a2343ff42966a02 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:08:53 -0700 Subject: [PATCH 02/13] First hack at being able to return NSImage pointers --- atom/common/api/atom_api_native_image.cc | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 69ead0464585..aac11fdfaaf1 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -58,6 +58,20 @@ ScaleFactorPair kScaleFactorPairs[] = { { "@2.5x" , 2.5f }, }; +enum NativeRepresentation { + INVALID = 0, + AS_NSIMAGE, +}; + +struct NativeRepresentationPair { + const char* name; + NativeRepresentation rep; +}; + +NativeRepresentationPair kNativeRepresentations[] { + { "nsimage", NativeRepresentation::AS_NSIMAGE }, +}; + float GetScaleFactorFromPath(const base::FilePath& path) { std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe()); @@ -184,6 +198,7 @@ mate::ObjectTemplateBuilder NativeImage::GetObjectTemplateBuilder( template_.Reset(isolate, mate::ObjectTemplateBuilder(isolate) .SetMethod("toPng", &NativeImage::ToPNG) .SetMethod("toJpeg", &NativeImage::ToJPEG) + .SetMethod("asNativeRepresentation", &NativeImage::AsNativeRepresentation) .SetMethod("toDataURL", &NativeImage::ToDataURL) .SetMethod("toDataUrl", &NativeImage::ToDataURL) // deprecated. .SetMethod("isEmpty", &NativeImage::IsEmpty) @@ -221,6 +236,47 @@ std::string NativeImage::ToDataURL() { return data_url; } +v8::Local NativeImage::AsNativeRepresentation(v8::Isolate* isolate, mate::Arguments* args) { + NativeRepresentation desiredRep = NativeRepresentation::INVALID; + void* ptr = nullptr; + std::string type; + + if (!args->GetNext(&type)) { + args->ThrowError(); + goto out; + } + + for (const NativeRepresentationPair& item : kNativeRepresentations) { + if (type.compare(item.name) == 0) { + desiredRep = item.rep; + break; + } + } + + if (desiredRep == NativeRepresentation::INVALID) { + args->ThrowError(); + goto out; + } + + switch (desiredRep) { +#if defined(OS_MACOSX) + case NativeRepresentation::AS_NSIMAGE: + ptr = reinterpret_cast(image_.AsNSImage()); + break; +#endif + default: + args->ThrowError(); + break; + } + +out: + + return node::Buffer::Copy( + isolate, + reinterpret_cast(ptr), + sizeof(void*)).ToLocalChecked(); +} + bool NativeImage::IsEmpty() { return image_.IsEmpty(); } From 5dea4b9b1cdc402e36749aa64b2e6e515e04f5fb Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:11:43 -0700 Subject: [PATCH 03/13] Add documentation --- docs/api/native-image.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/api/native-image.md b/docs/api/native-image.md index 515e89fba1a4..2cae12feb472 100644 --- a/docs/api/native-image.md +++ b/docs/api/native-image.md @@ -133,6 +133,13 @@ Returns a [Buffer][buffer] that contains the image's `JPEG` encoded data. Returns the data URL of the image. +### `image.asNativeRepresentation(type)` + +Returns a pointer to an underlying native type (encoded as a [Buffer][buffer]) which can be used with native APIs. Note that in many cases, this pointer is a weak pointer to the underlying native image not a copy, so you _must_ ensure that the associated `nativeImage` instance is kept around. + +* `type` String (**required**) - one of: + * `nsimage` - (OS X only) a pointer to an `NSImage` + ### `image.isEmpty()` Returns a boolean whether the image is empty. From 7233c83874e6444a96045bffe511c87bf866361f Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:18:03 -0700 Subject: [PATCH 04/13] Linting --- atom/common/api/atom_api_native_image.cc | 19 +++++++++++-------- atom/common/api/atom_api_native_image.h | 4 +++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index aac11fdfaaf1..7a84f999949c 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -198,7 +198,8 @@ mate::ObjectTemplateBuilder NativeImage::GetObjectTemplateBuilder( template_.Reset(isolate, mate::ObjectTemplateBuilder(isolate) .SetMethod("toPng", &NativeImage::ToPNG) .SetMethod("toJpeg", &NativeImage::ToJPEG) - .SetMethod("asNativeRepresentation", &NativeImage::AsNativeRepresentation) + .SetMethod("asNativeRepresentation", + &NativeImage::AsNativeRepresentation) .SetMethod("toDataURL", &NativeImage::ToDataURL) .SetMethod("toDataUrl", &NativeImage::ToDataURL) // deprecated. .SetMethod("isEmpty", &NativeImage::IsEmpty) @@ -236,28 +237,30 @@ std::string NativeImage::ToDataURL() { return data_url; } -v8::Local NativeImage::AsNativeRepresentation(v8::Isolate* isolate, mate::Arguments* args) { +v8::Local NativeImage::AsNativeRepresentation( + v8::Isolate* isolate, + mate::Arguments* args) { NativeRepresentation desiredRep = NativeRepresentation::INVALID; void* ptr = nullptr; std::string type; - + if (!args->GetNext(&type)) { args->ThrowError(); goto out; } - + for (const NativeRepresentationPair& item : kNativeRepresentations) { if (type.compare(item.name) == 0) { desiredRep = item.rep; break; } } - + if (desiredRep == NativeRepresentation::INVALID) { args->ThrowError(); goto out; } - + switch (desiredRep) { #if defined(OS_MACOSX) case NativeRepresentation::AS_NSIMAGE: @@ -268,9 +271,9 @@ v8::Local NativeImage::AsNativeRepresentation(v8::Isolate* isolate, m args->ThrowError(); break; } - + out: - + return node::Buffer::Copy( isolate, reinterpret_cast(ptr), diff --git a/atom/common/api/atom_api_native_image.h b/atom/common/api/atom_api_native_image.h index 8bc213040415..8b1329d28481 100644 --- a/atom/common/api/atom_api_native_image.h +++ b/atom/common/api/atom_api_native_image.h @@ -61,7 +61,9 @@ class NativeImage : public mate::Wrappable { private: v8::Local ToPNG(v8::Isolate* isolate); v8::Local ToJPEG(v8::Isolate* isolate, int quality); - v8::Local AsNativeRepresentation(v8::Isolate* isolate, mate::Arguments* args); + v8::Local AsNativeRepresentation( + v8::Isolate* isolate, + mate::Arguments* args); std::string ToDataURL(); bool IsEmpty(); gfx::Size GetSize(); From 248ac5c37b6eeeaefca70d32cbc3adf737a7da93 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:25:49 -0700 Subject: [PATCH 05/13] Add unit tests --- spec/api-native-image-spec.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 043a914578c0..587ea876f29a 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -34,5 +34,32 @@ describe('nativeImage module', () => { assert.equal(image.getSize().height, 190); assert.equal(image.getSize().width, 538); }); + + it('Gets an NSImage pointer on OS X', () => { + if (process.platform !== 'darwin') return; + + const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; + const image = nativeImage.createFromPath(imagePath); + const nsimage = image.asNativeRepresentation('nsimage'); + + assert.equal(nsimage.length, 8); + + // If all bytes are null, that's Bad + assert.equal(nsimage.reduce((acc,x) => acc || (x != 0)), true); + }); + + it('Throws when asNativeRepresentation gets a bogus value', () => { + const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; + const image = nativeImage.createFromPath(imagePath); + + let shouldDie = true; + try { + image.asNativeRepresentation('__foobar__'); + } catch (e) { + shouldDie = false; + } + + assert.equal(shouldDie, false); + }); }); }); From 63d917482226d5ce86038262fcd1993604883bbd Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Sun, 13 Mar 2016 20:27:44 -0700 Subject: [PATCH 06/13] :fire: build warning on Win32 --- atom/common/api/atom_api_native_image.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 7a84f999949c..6c22830f6748 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -267,6 +267,7 @@ v8::Local NativeImage::AsNativeRepresentation( ptr = reinterpret_cast(image_.AsNSImage()); break; #endif + case NativeRepresentation::INVALID: default: args->ThrowError(); break; From e3e6cd6fd8d3b55677be9972c38a8dcaf5ea6a7d Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Mon, 14 Mar 2016 19:48:40 -0700 Subject: [PATCH 07/13] Remove type parameter --- atom/common/api/atom_api_native_image.cc | 50 ++---------------------- 1 file changed, 4 insertions(+), 46 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 6c22830f6748..3932921646ef 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -58,20 +58,6 @@ ScaleFactorPair kScaleFactorPairs[] = { { "@2.5x" , 2.5f }, }; -enum NativeRepresentation { - INVALID = 0, - AS_NSIMAGE, -}; - -struct NativeRepresentationPair { - const char* name; - NativeRepresentation rep; -}; - -NativeRepresentationPair kNativeRepresentations[] { - { "nsimage", NativeRepresentation::AS_NSIMAGE }, -}; - float GetScaleFactorFromPath(const base::FilePath& path) { std::string filename(path.BaseName().RemoveExtension().AsUTF8Unsafe()); @@ -240,40 +226,12 @@ std::string NativeImage::ToDataURL() { v8::Local NativeImage::AsNativeRepresentation( v8::Isolate* isolate, mate::Arguments* args) { - NativeRepresentation desiredRep = NativeRepresentation::INVALID; - void* ptr = nullptr; - std::string type; - - if (!args->GetNext(&type)) { - args->ThrowError(); - goto out; - } - - for (const NativeRepresentationPair& item : kNativeRepresentations) { - if (type.compare(item.name) == 0) { - desiredRep = item.rep; - break; - } - } - - if (desiredRep == NativeRepresentation::INVALID) { - args->ThrowError(); - goto out; - } - - switch (desiredRep) { #if defined(OS_MACOSX) - case NativeRepresentation::AS_NSIMAGE: - ptr = reinterpret_cast(image_.AsNSImage()); - break; + void* ptr = reinterpret_cast(image_.AsNSImage()); +#else + args.ThrowError(); + return v8::Undefined(isolate); #endif - case NativeRepresentation::INVALID: - default: - args->ThrowError(); - break; - } - -out: return node::Buffer::Copy( isolate, From d344c1e408324a39c703183c7541ea444d5ba18f Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Mon, 14 Mar 2016 19:50:31 -0700 Subject: [PATCH 08/13] AsNativeRepresentation => getNativeHandle --- atom/common/api/atom_api_native_image.cc | 6 +++--- atom/common/api/atom_api_native_image.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index 3932921646ef..d616b4283302 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -184,8 +184,8 @@ mate::ObjectTemplateBuilder NativeImage::GetObjectTemplateBuilder( template_.Reset(isolate, mate::ObjectTemplateBuilder(isolate) .SetMethod("toPng", &NativeImage::ToPNG) .SetMethod("toJpeg", &NativeImage::ToJPEG) - .SetMethod("asNativeRepresentation", - &NativeImage::AsNativeRepresentation) + .SetMethod("getNativeHandle", + &NativeImage::GetNativeHandle) .SetMethod("toDataURL", &NativeImage::ToDataURL) .SetMethod("toDataUrl", &NativeImage::ToDataURL) // deprecated. .SetMethod("isEmpty", &NativeImage::IsEmpty) @@ -223,7 +223,7 @@ std::string NativeImage::ToDataURL() { return data_url; } -v8::Local NativeImage::AsNativeRepresentation( +v8::Local NativeImage::GetNativeHandle( v8::Isolate* isolate, mate::Arguments* args) { #if defined(OS_MACOSX) diff --git a/atom/common/api/atom_api_native_image.h b/atom/common/api/atom_api_native_image.h index 8b1329d28481..145f5ff1dcdc 100644 --- a/atom/common/api/atom_api_native_image.h +++ b/atom/common/api/atom_api_native_image.h @@ -61,7 +61,7 @@ class NativeImage : public mate::Wrappable { private: v8::Local ToPNG(v8::Isolate* isolate); v8::Local ToJPEG(v8::Isolate* isolate, int quality); - v8::Local AsNativeRepresentation( + v8::Local GetNativeHandle( v8::Isolate* isolate, mate::Arguments* args); std::string ToDataURL(); From f59752bf4f7829bdcd3f0cd967938afea9bf62c6 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Mon, 14 Mar 2016 19:51:37 -0700 Subject: [PATCH 09/13] Update the docs to match --- docs/api/native-image.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/api/native-image.md b/docs/api/native-image.md index 2cae12feb472..c33f35c6fe51 100644 --- a/docs/api/native-image.md +++ b/docs/api/native-image.md @@ -133,12 +133,11 @@ Returns a [Buffer][buffer] that contains the image's `JPEG` encoded data. Returns the data URL of the image. -### `image.asNativeRepresentation(type)` +### `image.getNativeHandle()` Returns a pointer to an underlying native type (encoded as a [Buffer][buffer]) which can be used with native APIs. Note that in many cases, this pointer is a weak pointer to the underlying native image not a copy, so you _must_ ensure that the associated `nativeImage` instance is kept around. -* `type` String (**required**) - one of: - * `nsimage` - (OS X only) a pointer to an `NSImage` +Returns a [Buffer][buffer] that represents a pointer to a native type - on OS X, this type is an NSImage object. ### `image.isEmpty()` From 665d3166ed02a3d169f69d3a52cd5fad2996803e Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Mon, 14 Mar 2016 21:00:58 -0700 Subject: [PATCH 10/13] Update the tests --- spec/api-native-image-spec.js | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index 587ea876f29a..e411fb110026 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -40,26 +40,12 @@ describe('nativeImage module', () => { const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; const image = nativeImage.createFromPath(imagePath); - const nsimage = image.asNativeRepresentation('nsimage'); + const nsimage = image.getNativeHandle(); assert.equal(nsimage.length, 8); // If all bytes are null, that's Bad assert.equal(nsimage.reduce((acc,x) => acc || (x != 0)), true); }); - - it('Throws when asNativeRepresentation gets a bogus value', () => { - const imagePath = `${path.join(__dirname, 'fixtures', 'api')}${path.sep}..${path.sep}${path.join('assets', 'logo.png')}`; - const image = nativeImage.createFromPath(imagePath); - - let shouldDie = true; - try { - image.asNativeRepresentation('__foobar__'); - } catch (e) { - shouldDie = false; - } - - assert.equal(shouldDie, false); - }); }); }); From fa197ad5832b43044f6731c83e278d4bea3b83cd Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Wed, 16 Mar 2016 12:40:28 -0700 Subject: [PATCH 11/13] Fix failing test --- spec/api-native-image-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index e411fb110026..fd13ad76db30 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -45,7 +45,7 @@ describe('nativeImage module', () => { assert.equal(nsimage.length, 8); // If all bytes are null, that's Bad - assert.equal(nsimage.reduce((acc,x) => acc || (x != 0)), true); + assert.equal(!!nsimage.reduce((acc,x) => acc || (x != 0)), true); }); }); }); From 148014f99a8a764b795233299cf0e6f4478c15ac Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Wed, 16 Mar 2016 12:42:23 -0700 Subject: [PATCH 12/13] Fix spec failure --- spec/api-native-image-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-native-image-spec.js b/spec/api-native-image-spec.js index fd13ad76db30..a7ee8be007ed 100644 --- a/spec/api-native-image-spec.js +++ b/spec/api-native-image-spec.js @@ -45,7 +45,7 @@ describe('nativeImage module', () => { assert.equal(nsimage.length, 8); // If all bytes are null, that's Bad - assert.equal(!!nsimage.reduce((acc,x) => acc || (x != 0)), true); + assert.equal(nsimage.reduce((acc,x) => acc || (x != 0), false), true); }); }); }); From e94da877c2b3a205d196ee5dc5781e5b9524cc26 Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Wed, 16 Mar 2016 12:49:34 -0700 Subject: [PATCH 13/13] Fix compile oopses on non-OS X --- atom/common/api/atom_api_native_image.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/atom/common/api/atom_api_native_image.cc b/atom/common/api/atom_api_native_image.cc index d616b4283302..81db35715bcd 100644 --- a/atom/common/api/atom_api_native_image.cc +++ b/atom/common/api/atom_api_native_image.cc @@ -226,10 +226,11 @@ std::string NativeImage::ToDataURL() { v8::Local NativeImage::GetNativeHandle( v8::Isolate* isolate, mate::Arguments* args) { +void* ptr = NULL; #if defined(OS_MACOSX) - void* ptr = reinterpret_cast(image_.AsNSImage()); + ptr = reinterpret_cast(image_.AsNSImage()); #else - args.ThrowError(); + args->ThrowError(); return v8::Undefined(isolate); #endif