New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bitmap support to nativeImage.createFromBuffer #8175

Merged
merged 7 commits into from Dec 14, 2016

Conversation

Projects
None yet
4 participants
@gerhardberger
Member

gerhardberger commented Dec 9, 2016

This PR adds support to nativeImage.createFromBuffer for creating images from bitmap buffers. #7950

We cannot obtain size from the buffer, it has to be supplied as well, so I added two optional parameters to the method.

Show outdated Hide outdated docs/api/native-image.md

Fine with it as long as it is not breaking

@gerhardberger

This comment has been minimized.

Show comment
Hide comment
@gerhardberger

gerhardberger Dec 13, 2016

Member

As @kevinsawicki suggested, I moved the width and height parameters to an options object as a last optional parameter, which can also contain the scaleFactor.

Member

gerhardberger commented Dec 13, 2016

As @kevinsawicki suggested, I moved the width and height parameters to an options object as a last optional parameter, which can also contain the scaleFactor.

@@ -25,6 +25,10 @@ describe('nativeImage module', () => {
const imageC = nativeImage.createFromBuffer(imageA.toJPEG(100))
assert.deepEqual(imageC.getSize(), {width: 538, height: 190})
const imageD = nativeImage.createFromBuffer(imageA.toBitmap(),
{width: 538, height: 190})

This comment has been minimized.

@kevinsawicki

kevinsawicki Dec 13, 2016

Contributor

Looks like this image is initially 538x190 so could this spec use different values for the specified width/height to ensure a different size is returned?

@kevinsawicki

kevinsawicki Dec 13, 2016

Contributor

Looks like this image is initially 538x190 so could this spec use different values for the specified width/height to ensure a different size is returned?

This comment has been minimized.

@gerhardberger

gerhardberger Dec 13, 2016

Member

Do you mean to test for a different image or to create a falsy assertion with wrong width/height values?

@gerhardberger

gerhardberger Dec 13, 2016

Member

Do you mean to test for a different image or to create a falsy assertion with wrong width/height values?

This comment has been minimized.

@kevinsawicki

kevinsawicki Dec 13, 2016

Contributor

Yeah, what happens if the specified width/height doesn't match the bitmap? Does it fail to return it? Or return an empty buffer? Or something else?

Would be good to cover this case to know the behavior.

@kevinsawicki

kevinsawicki Dec 13, 2016

Contributor

Yeah, what happens if the specified width/height doesn't match the bitmap? Does it fail to return it? Or return an empty buffer? Or something else?

Would be good to cover this case to know the behavior.

This comment has been minimized.

@gerhardberger

gerhardberger Dec 13, 2016

Member

I added some tests for bitmaps and the other types. PNG and JPEG is not affected by width/height, because the size can be decoded from the buffer, but bitmaps don't have that so they always have the same size, that is specified. That said they won't crash when a wrong size is passed, just the picture will be messed up.

@gerhardberger

gerhardberger Dec 13, 2016

Member

I added some tests for bitmaps and the other types. PNG and JPEG is not affected by width/height, because the size can be decoded from the buffer, but bitmaps don't have that so they always have the same size, that is specified. That said they won't crash when a wrong size is passed, just the picture will be messed up.

@kevinsawicki kevinsawicki merged commit cd067bc into electron:master Dec 14, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Dec 14, 2016

Contributor

Thanks for this @gerhardberger 👍 🚢

Contributor

kevinsawicki commented Dec 14, 2016

Thanks for this @gerhardberger 👍 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment