Made CAIRO_FORMAT_RGB30 optional to support outdated versions of Cairo #315

Merged
merged 3 commits into from Oct 6, 2013

Projects

None yet

9 participants

@kkoopa
Contributor
kkoopa commented Aug 6, 2013

Fixes #312 and #314.

@kkoopa kkoopa referenced this pull request Aug 6, 2013
Closed

toBuffer optimization #306

Collaborator
kangax commented Aug 11, 2013

Fabric still failing on Ubuntu because of this. Will this be making it any time soon or should I downgrade from 1.1.0?

sehrgut commented Aug 25, 2013

Still failing on OS X with this. I'm having to run 1.0.4 because I can't build 1.1.0.

CXX(target) Release/obj.target/canvas/src/Canvas.o
In file included from ../src/Canvas.cc:8:
../src/PNG.h: In function ‘cairo_status_t canvas_write_png(cairo_surface_t*, void (*)(png_struct*, png_byte*, png_size_t), void*)’:
../src/PNG.h:139: error: ‘CAIRO_FORMAT_RGB30’ was not declared in this scope
../src/PNG.h:158: error: ‘CAIRO_FORMAT_INVALID’ was not declared in this scope
make: *** [Release/obj.target/canvas/src/Canvas.o] Error 1

As @kangax stated, fabric's NPM package's installation is failing because the C++ compiler throws an error while building the canvas package:

In file included from ../src/Canvas.cc:8:0:
../src/PNG.h: In function ‘cairo_status_t canvas_write_png(cairo_surface_t*, png_rw_ptr, void*)’:
../src/PNG.h:139:10: error: ‘CAIRO_FORMAT_RGB30’ was not declared in this scope
make: *** [Release/obj.target/canvas/src/Canvas.o] Error 1

I'm running Ubuntu 12.04LTS x64 on a VirtualBox machine. All packages listed as dependencies in your Wiki page were installed through apt-get:
libcairo2-dev : 1.10.2-6.1ubuntu3
libjpeg8-dev : 8c-2ubuntu7
libpango1.0-dev : 1.30.0-0ubuntu3.1
libgif-dev : 4.1.6-9ubuntu1

The NPM version of node-canvas is 1.1.0 on the NPM registry dated 25 days ago.

Should either node-canvas or even fabric be downgraded?

Collaborator
kangax commented Aug 26, 2013

@bernardofd I already downgraded to 1.0 coz we basically couldn't use travis.

@kangax You've updated your git repo, but the package.json file in fabric's NPM package is still downloading the node-canvas v1.1.0. Maybe you would need to publish a new version in the NPM registry?
Excerpt from the package.json in the NPM package:

"dependencies": {
    "canvas": "1.1.x",
    "jsdom": "0.7.x",
    "xmldom": "0.1.x"
},
Collaborator
kangax commented Aug 27, 2013

@bernardofd Good point. Pushed new fixed version to npm.

Collaborator
kangax commented Oct 1, 2013

@visionmedia @TooTallNate any reason this is not being pulled in? Lots of people having this issue.

@kangax kangax referenced this pull request in kangax/fabric.js Oct 4, 2013
Closed

Upgrade node-canvas version to 1.1 #887

👍 Please land this PR so Fabric can use the latest canvas awesome

Collaborator
rvagg commented Oct 4, 2013

Did @kangax end up getting repo access? Would be great since he has a lot of incentive for this to all work well.

@visionmedia ping

Collaborator
kangax commented Oct 4, 2013

@rvagg I do have access now, but is this definitely a safe thing to merge?

Collaborator
rvagg commented Oct 4, 2013

I don't know but I know I trust @kkoopa to do the right thing with C++.

Contributor
kkoopa commented Oct 6, 2013

It should be safe, I have tried it without running into problems. A buffer cannot be defined as having the format in question, without that format being defined and supported. So, old versions of Cairo should never see such a format in the first place, as it cannot generate them. An alternative would be to define CAIRO_FORMAT_RGB30 if it is not already defined, but then that case should still never be executed with an old version of Cairo, as such a buffer format should not be available.

@kangax kangax merged commit dca6b01 into Automattic:master Oct 6, 2013
freewil commented Oct 7, 2013

Great, this compiles now on Ubuntu 12.04 LTS. @kangax can we get a new version up on npm?

zuzusik commented Oct 7, 2013

Yes, publish please to npm registry and thanks a lot for a fix!

Collaborator
kangax commented Oct 9, 2013

@freewil @zuzusik @superjoe30 I tagged version 1.1.1 but need @visionmedia to publish to npm

Collaborator
kangax commented Oct 9, 2013

@freewil @zuzusik @superjoe30 pushed 1.1.1 to npm

andrewrk commented Oct 9, 2013

thanks!

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