Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 3dca82a

Browse files
committed
fix build / refactor backends
0. Most importantly this fixes the dumb mistake I made in the previous commit: not all backends implemented destroySurface. 1. Both Backend and Pdf/SvgBackend were cleaning up memory on exit. This responsibility should not be unclear; let's just do it all in the child class, since PdfBackend and SvgBackend have other stuff to cleanup. This is all still less code. 2. The only reason to destroy the surface is if it's dirty from e.g. setWidth (this is referring to isSurfaceValid) 3. Make createSurface idempotent. This allows us to merge it with getSurface() and makes it safer. Call it ensureSurface.
1 parent f5f103a commit 3dca82a

File tree

10 files changed

+60
-83
lines changed

10 files changed

+60
-83
lines changed

‎src/Canvas.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,13 @@ Canvas::Canvas(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Canvas>(info),
131131
if (instance.IsJust()) backend = ImageBackend::Unwrap(jsBackend = instance.Unwrap());
132132
}
133133

134+
backend->setCanvas(this);
135+
134136
if (!backend->isSurfaceValid()) {
135137
Napi::Error::New(env, backend->getError()).ThrowAsJavaScriptException();
136138
return;
137139
}
138140

139-
backend->setCanvas(this);
140-
141141
// Note: the backend gets destroyed when the jsBackend is GC'd. The cleaner
142142
// way would be to only store the jsBackend and unwrap it when the c++ ref is
143143
// needed, but that's slower and a burden. The _backend might be null if we
@@ -908,7 +908,7 @@ Canvas::resurface(Napi::Object This) {
908908

909909
if (This.Get("context").UnwrapTo(&context) && context.IsObject()) {
910910
backend()->destroySurface();
911-
backend()->createSurface();
911+
backend()->ensureSurface();
912912
// Reset context
913913
Context2d *context2d = Context2d::Unwrap(context.As<Napi::Object>());
914914
cairo_t *prev = context2d->context();

‎src/Canvas.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class Canvas : public Napi::ObjectWrap<Canvas> {
7676
static PangoFontDescription *ResolveFontDescription(const PangoFontDescription *desc);
7777

7878
DLL_PUBLIC inline Backend* backend() { return _backend; }
79-
DLL_PUBLIC inline cairo_surface_t* surface(){ return backend()->getSurface(); }
79+
DLL_PUBLIC inline cairo_surface_t* surface(){ return backend()->ensureSurface(); }
8080
cairo_t* createCairoContext();
8181

8282
DLL_PUBLIC inline uint8_t *data(){ return cairo_image_surface_get_data(surface()); }

‎src/backend/Backend.cc‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,31 +11,12 @@ Backend::Backend(std::string name, Napi::CallbackInfo& info) : name(name), env(i
1111
this->height = height;
1212
}
1313

14-
Backend::~Backend()
15-
{
16-
Backend::destroySurface();
17-
}
18-
1914
void Backend::setCanvas(Canvas* _canvas)
2015
{
2116
this->canvas = _canvas;
2217
}
2318

2419

25-
DLL_PUBLIC cairo_surface_t* Backend::getSurface() {
26-
if (!surface) createSurface();
27-
return surface;
28-
}
29-
30-
void Backend::destroySurface()
31-
{
32-
if(this->surface)
33-
{
34-
cairo_surface_destroy(this->surface);
35-
this->surface = NULL;
36-
}
37-
}
38-
3920

4021
std::string Backend::getName()
4122
{
@@ -50,7 +31,6 @@ void Backend::setWidth(int width_)
5031
{
5132
this->destroySurface();
5233
this->width = width_;
53-
this->createSurface();
5434
}
5535

5636
int Backend::getHeight()
@@ -61,23 +41,18 @@ void Backend::setHeight(int height_)
6141
{
6242
this->destroySurface();
6343
this->height = height_;
64-
this->createSurface();
6544
}
6645

67-
bool Backend::isSurfaceValid(){
68-
bool hadSurface = surface != NULL;
46+
bool Backend::isSurfaceValid() {
6947
bool isValid = true;
7048

71-
cairo_status_t status = cairo_surface_status(getSurface());
49+
cairo_status_t status = cairo_surface_status(ensureSurface());
7250

7351
if (status != CAIRO_STATUS_SUCCESS) {
7452
error = cairo_status_to_string(status);
7553
isValid = false;
7654
}
7755

78-
if (!hadSurface)
79-
destroySurface();
80-
8156
return isValid;
8257
}
8358

‎src/backend/Backend.h‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,17 @@ class Backend
1717
protected:
1818
int width;
1919
int height;
20-
cairo_surface_t* surface = nullptr;
2120
Canvas* canvas = nullptr;
2221

2322
Backend(std::string name, Napi::CallbackInfo& info);
2423

2524
public:
2625
Napi::Env env;
2726

28-
virtual ~Backend();
29-
3027
void setCanvas(Canvas* canvas);
3128

32-
virtual cairo_surface_t* createSurface() = 0;
33-
34-
DLL_PUBLIC cairo_surface_t* getSurface();
35-
virtual void destroySurface();
29+
virtual cairo_surface_t* ensureSurface() = 0;
30+
virtual void destroySurface() = 0;
3631

3732
DLL_PUBLIC std::string getName();
3833

‎src/backend/ImageBackend.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
#include <napi.h>
44
#include <cassert>
55

6-
ImageBackend::ImageBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<ImageBackend>(info), Backend("image", info)
7-
{
6+
ImageBackend::ImageBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<ImageBackend>(info), Backend("image", info) {}
7+
8+
ImageBackend::~ImageBackend() {
9+
destroySurface();
810
}
911

1012
// This returns an approximate value only, suitable for
@@ -29,11 +31,12 @@ int32_t ImageBackend::approxBytesPerPixel() {
2931
}
3032
}
3133

32-
cairo_surface_t* ImageBackend::createSurface() {
33-
assert(!surface);
34-
surface = cairo_image_surface_create(format, width, height);
35-
assert(surface);
36-
Napi::MemoryManagement::AdjustExternalMemory(env, approxBytesPerPixel() * width * height);
34+
cairo_surface_t* ImageBackend::ensureSurface() {
35+
if (!surface) {
36+
surface = cairo_image_surface_create(format, width, height);
37+
assert(surface);
38+
Napi::MemoryManagement::AdjustExternalMemory(env, approxBytesPerPixel() * width * height);
39+
}
3740
return surface;
3841
}
3942

@@ -50,7 +53,8 @@ cairo_format_t ImageBackend::getFormat() {
5053
}
5154

5255
void ImageBackend::setFormat(cairo_format_t _format) {
53-
this->format = _format;
56+
this->destroySurface();
57+
this->format = _format;
5458
}
5559

5660
Napi::FunctionReference ImageBackend::constructor;

‎src/backend/ImageBackend.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@
66
class ImageBackend : public Napi::ObjectWrap<ImageBackend>, public Backend
77
{
88
private:
9-
cairo_surface_t* createSurface();
9+
cairo_surface_t* ensureSurface();
1010
void destroySurface();
1111
cairo_format_t format = DEFAULT_FORMAT;
12+
cairo_surface_t* surface = nullptr;
1213

1314
public:
1415
ImageBackend(Napi::CallbackInfo& info);
16+
~ImageBackend();
1517

1618
cairo_format_t getFormat();
1719
void setFormat(cairo_format_t format);

‎src/backend/PdfBackend.cc‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,29 @@
55
#include "../Canvas.h"
66
#include "../closure.h"
77

8-
PdfBackend::PdfBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<PdfBackend>(info), Backend("pdf", info) {
9-
PdfBackend::createSurface();
10-
}
8+
PdfBackend::PdfBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<PdfBackend>(info), Backend("pdf", info) {}
119

1210
PdfBackend::~PdfBackend() {
13-
cairo_surface_finish(surface);
14-
if (_closure) delete _closure;
1511
destroySurface();
1612
}
1713

18-
cairo_surface_t* PdfBackend::createSurface() {
19-
if (!_closure) _closure = new PdfSvgClosure(canvas);
20-
surface = cairo_pdf_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
14+
cairo_surface_t* PdfBackend::ensureSurface() {
15+
if (!surface) {
16+
_closure = new PdfSvgClosure(canvas);
17+
surface = cairo_pdf_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
18+
}
2119
return surface;
2220
}
2321

24-
cairo_surface_t* PdfBackend::recreateSurface() {
25-
cairo_pdf_surface_set_size(surface, width, height);
26-
27-
return surface;
22+
void PdfBackend::destroySurface() {
23+
if (surface) {
24+
cairo_surface_destroy(surface);
25+
surface = nullptr;
26+
if (_closure) {
27+
delete _closure;
28+
_closure = nullptr;
29+
}
30+
}
2831
}
2932

3033
void

‎src/backend/PdfBackend.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
class PdfBackend : public Napi::ObjectWrap<PdfBackend>, public Backend
88
{
99
private:
10-
cairo_surface_t* createSurface();
11-
cairo_surface_t* recreateSurface();
10+
cairo_surface_t* ensureSurface();
11+
void destroySurface();
12+
cairo_surface_t* surface = nullptr;
1213

1314
public:
1415
PdfSvgClosure* _closure = NULL;

‎src/backend/SvgBackend.cc‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,32 @@
99

1010
using namespace Napi;
1111

12-
SvgBackend::SvgBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<SvgBackend>(info), Backend("svg", info) {
13-
SvgBackend::createSurface();
14-
}
12+
SvgBackend::SvgBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<SvgBackend>(info), Backend("svg", info) {}
1513

1614
SvgBackend::~SvgBackend() {
17-
cairo_surface_finish(surface);
18-
if (_closure) {
19-
delete _closure;
20-
_closure = nullptr;
21-
}
2215
destroySurface();
2316
}
2417

25-
cairo_surface_t* SvgBackend::createSurface() {
26-
assert(!_closure);
27-
_closure = new PdfSvgClosure(canvas);
28-
surface = cairo_svg_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
18+
cairo_surface_t* SvgBackend::ensureSurface() {
19+
if (!surface) {
20+
assert(!_closure);
21+
_closure = new PdfSvgClosure(canvas);
22+
surface = cairo_svg_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
23+
}
2924
return surface;
3025
}
3126

32-
cairo_surface_t* SvgBackend::recreateSurface() {
33-
cairo_surface_finish(surface);
34-
delete _closure;
35-
_closure = nullptr;
36-
cairo_surface_destroy(surface);
37-
38-
return createSurface();
27+
void SvgBackend::destroySurface() {
28+
if (surface) {
29+
cairo_surface_destroy(surface);
30+
surface = nullptr;
31+
if (_closure) {
32+
delete _closure;
33+
_closure = nullptr;
34+
}
35+
}
3936
}
4037

41-
4238
void
4339
SvgBackend::Initialize(Napi::Object target) {
4440
Napi::Env env = target.Env();

‎src/backend/SvgBackend.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
class SvgBackend : public Napi::ObjectWrap<SvgBackend>, public Backend
88
{
99
private:
10-
cairo_surface_t* createSurface();
11-
cairo_surface_t* recreateSurface();
10+
cairo_surface_t* ensureSurface();
11+
void destroySurface();
12+
cairo_surface_t* surface = nullptr;
1213

1314
public:
1415
PdfSvgClosure* _closure = NULL;

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /