Skip to content
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.

Sign up for GitHub

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 support for context globalAlpha for gradients and patterns #1064

Merged
merged 16 commits into from
Dec 28, 2017
Merged

Add support for context globalAlpha for gradients and patterns #1064

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5875be3
sofar so good
asturur Dec 20, 2017
b5f6545
Merge pull request #1 from Automattic/v1.x
asturur Dec 21, 2017
74e9250
Merge branch 'v1.x' into gradients-opacity
asturur Dec 21, 2017
e00aa8b
this is a draft
asturur Dec 21, 2017
009c632
different fix
asturur Dec 21, 2017
e3cdd85
partial update
asturur Dec 22, 2017
931c0d3
should be doone
asturur Dec 22, 2017
ac4dd37
Merge pull request #2 from Automattic/v1.x
asturur Dec 22, 2017
5e9b4e6
merged conflicts
asturur Dec 22, 2017
93dab0c
fix breks
asturur Dec 22, 2017
a5c2e74
different handling
asturur Dec 24, 2017
b2be100
removed non more valid comments;
asturur Dec 24, 2017
5e5614a
working error management
asturur Dec 26, 2017
c894c8b
return from fill/stroke if new pattern failed to allocate
asturur Dec 27, 2017
e679840
Merge branch 'v1.x' into gradients-opacity
asturur Dec 27, 2017
803a24e
merged master and used NULL
asturur Dec 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CanvasPattern.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ NAN_METHOD(Pattern::New) {


/*
* Initialize linear gradient.
* Initialize pattern.
*/

Pattern::Pattern(cairo_surface_t *surface) {
Expand Down
102 changes: 96 additions & 6 deletions src/CanvasRenderingContext2d.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,66 @@ Context2d::restorePath() {
cairo_path_destroy(_path);
}

/*
* Create temporary surface for gradient or pattern transparency
*/
cairo_pattern_t*
create_transparent_gradient(cairo_pattern_t *source, float alpha) {
double x0;
double y0;
double x1;
double y1;
double r0;
double r1;
int count;
int i;
double offset;
double r;
double g;
double b;
double a;
cairo_pattern_t *newGradient;
cairo_pattern_type_t type = cairo_pattern_get_type(source);
cairo_pattern_get_color_stop_count(source, &count);
if (type == CAIRO_PATTERN_TYPE_LINEAR) {
cairo_pattern_get_linear_points (source, &x0, &y0, &x1, &y1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no space between fn name and parentheses
same a few lines down (322)

newGradient = cairo_pattern_create_linear(x0, y0, x1, y1);
}
if (type == CAIRO_PATTERN_TYPE_RADIAL) {
cairo_pattern_get_radial_circles(source, &x0, &y0, &r0, &x1, &y1, &r1);
newGradient = cairo_pattern_create_radial(x0, y0, r0, x1, y1, r1);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get rid of the maybe-uninitialized warning here, can you make this if (linear) {} else if (raidal) {} else { do something reasonable like throwing an error } please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i did not get any warning, but definitely i can guard against it.

for ( i = 0; i < count; i++ ) {
cairo_pattern_get_color_stop_rgba(source, i, &offset, &r, &g, &b, &a);
cairo_pattern_add_color_stop_rgba(newGradient, offset, r, g, b, a * alpha);
}
return newGradient;
}

cairo_pattern_t*
create_transparent_pattern(cairo_pattern_t *source, float alpha) {
cairo_surface_t *surface;
cairo_pattern_get_surface(source, &surface);
int width = cairo_image_surface_get_width(surface);
int height = cairo_image_surface_get_height(surface);
cairo_surface_t *mask_surface = cairo_image_surface_create(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*mask_surface needs to be freed at some point. I think it's safe to call cairo_surface_destroy just after your cairo_pattern_create_for_surface(mask_surface) call, so you can do memory management within this function.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't see this being freed... calling cairo_surface_destroy decreases the ref count, but doesn't destroy it unless the ref count is 0. cairo_pattern_create_for_surface increases the ref count on the surface. There's an example of this exact lifecycle pattern in the Cairo source code here. Make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought that since the pattern is alive and to be used you have to keep the surface.
I get this feeling from the sentence 'the pattern owns the surface' but maybe i m wrong. I'll try to delete everything, if it still draws is ok.

CAIRO_FORMAT_ARGB32,
width,
height);
cairo_t *mask_context = cairo_create(mask_surface);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*mask_context needs to be freed. Looks like you don't need it after cairo_paint_with_alpha so that's easy.

You should also check cairo_status(mask_context) since this could fail to allocate.

if (cairo_status(mask_context) != CAIRO_STATUS_SUCCESS) {
// context creation failed. Return the original source and destroy the surface
cairo_surface_destroy(mask_surface);
return source;
}
cairo_set_source(mask_context, source);
cairo_paint_with_alpha(mask_context, alpha);
// remove the context, but not the surface since is owned by the pattern
cairo_destroy(mask_context);
cairo_pattern_t* newPattern = cairo_pattern_create_for_surface(mask_surface);
return newPattern;
}

/*
* Fill and apply shadow.
*/
Expand All @@ -310,17 +370,29 @@ Context2d::setFillRule(v8::Local<v8::Value> value) {

void
Context2d::fill(bool preserve) {
cairo_pattern_t *new_pattern = NULL;
if (state->fillPattern) {
cairo_set_source(_context, state->fillPattern);
if (state->globalAlpha < 1) {
new_pattern = create_transparent_pattern(state->fillPattern, state->globalAlpha);
cairo_set_source(_context, new_pattern);
} else {
cairo_set_source(_context, state->fillPattern);
}
cairo_pattern_set_extend(cairo_get_source(_context), CAIRO_EXTEND_REPEAT);
// TODO repeat/repeat-x/repeat-y
} else if (state->fillGradient) {
cairo_pattern_set_filter(state->fillGradient, state->patternQuality);
cairo_set_source(_context, state->fillGradient);
if (state->globalAlpha < 1) {
new_pattern = create_transparent_gradient(state->fillGradient, state->globalAlpha);
cairo_pattern_set_filter(new_pattern, state->patternQuality);
cairo_set_source(_context, new_pattern);
} else {
cairo_pattern_set_filter(state->fillGradient, state->patternQuality);
cairo_set_source(_context, state->fillGradient);
}
} else {
setSourceRGBA(state->fill);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment far down enough, but after the shadow/fill calls a few lines down, you need to free the new_pattern or new_gradient cairo_pattern_ts with https://developer.gnome.org/cairo/stable/cairo-cairo-pattern-t.html#cairo-pattern-destroy.

if (preserve) {
hasShadow()
? shadow(cairo_fill_preserve)
Expand All @@ -330,6 +402,9 @@ Context2d::fill(bool preserve) {
? shadow(cairo_fill)
: cairo_fill(_context);
}
if (new_pattern != state->fillPattern && new_pattern != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new_pattern != state->fillPattern part of this makes the cairo_pattern_destroy unreachable in cases of global alpha < 1. Here again the destroy fn actually just decreases ref count; it should be safe to call it whenever new_pattern != NULL.

Actually, you can make the destroy call after line 388 I think, right after cairo_set_source(_context, new_pattern). That'll let you shrink the scope of new_pattern as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added this check because the new_pattern can be NULL globalAlpha = 1 or can be fillPattern if the creation of the new pattern failed ( because we return the same pattern from the other function ).
Why you think is unreachable?

cairo_pattern_destroy(new_pattern);
}
}

/*
Expand All @@ -338,12 +413,24 @@ Context2d::fill(bool preserve) {

void
Context2d::stroke(bool preserve) {
cairo_pattern_t *new_pattern = NULL;
if (state->strokePattern) {
cairo_set_source(_context, state->strokePattern);
if (state->globalAlpha < 1) {
new_pattern = create_transparent_pattern(state->strokePattern, state->globalAlpha);
cairo_set_source(_context, new_pattern);
} else {
cairo_set_source(_context, state->strokePattern);
}
cairo_pattern_set_extend(cairo_get_source(_context), CAIRO_EXTEND_REPEAT);
} else if (state->strokeGradient) {
cairo_pattern_set_filter(state->strokeGradient, state->patternQuality);
cairo_set_source(_context, state->strokeGradient);
if (state->globalAlpha < 1) {
new_pattern = create_transparent_gradient(state->strokeGradient, state->globalAlpha);
cairo_pattern_set_filter(new_pattern, state->patternQuality);
cairo_set_source(_context, new_pattern);
} else {
cairo_pattern_set_filter(state->strokeGradient, state->patternQuality);
cairo_set_source(_context, state->strokeGradient);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto about freeing the cairo_pattern_ts at the end of this function.

} else {
setSourceRGBA(state->stroke);
}
Expand All @@ -357,6 +444,9 @@ Context2d::stroke(bool preserve) {
? shadow(cairo_stroke)
: cairo_stroke(_context);
}
if (new_pattern != state->strokePattern && new_pattern != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

cairo_pattern_destroy(new_pattern);
}
}

/*
Expand Down
93 changes: 93 additions & 0 deletions test/public/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,38 @@ tests['clip() 2'] = function(ctx){
}
};

tests['createPattern()'] = function(ctx, done) {
var img = new Image;
img.onload = function(){
var pattern = ctx.createPattern(img, 'repeat');
ctx.scale(0.1, 0.1);
ctx.fillStyle = pattern;
ctx.fillRect(100, 100, 800, 800);
ctx.strokeStyle = pattern;
ctx.lineWidth = 200
ctx.strokeRect(1100, 1100, 800, 800);
done();
};
img.src = 'face.jpeg';
};

tests['createPattern() with globalAlpha'] = function(ctx, done) {
var img = new Image;
img.onload = function(){
var pattern = ctx.createPattern(img, 'repeat');
ctx.scale(0.1, 0.1);
ctx.globalAlpha = 0.6;
ctx.fillStyle = pattern;
ctx.fillRect(100, 100, 800, 800);
ctx.globalAlpha = 0.2;
ctx.strokeStyle = pattern;
ctx.lineWidth = 200
ctx.strokeRect(1100, 1100, 800, 800);
done();
};
img.src = 'face.jpeg';
};

tests['createLinearGradient()'] = function(ctx){
var lingrad = ctx.createLinearGradient(0,0,0,150);
lingrad.addColorStop(0, '#00ABEB');
Expand All @@ -391,6 +423,56 @@ tests['createLinearGradient()'] = function(ctx){
ctx.fillStyle = '#13b575';
ctx.fillStyle = ctx.fillStyle;
ctx.fillRect(65,65,20,20);

var lingrad = ctx.createLinearGradient(0,0,200,0);
lingrad.addColorStop(0, 'rgba(0,255,0,0.5)');
lingrad.addColorStop(0.33, 'rgba(255,255,0,0.5)');
lingrad.addColorStop(0.66, 'rgba(0,255,255,0.5)');
lingrad.addColorStop(1, 'rgba(255,0,255,0.5)');
ctx.fillStyle = lingrad;
ctx.fillRect(0,170,200,30);
};

tests['createLinearGradient() with opacity'] = function(ctx){
var lingrad = ctx.createLinearGradient(0,0,0,200);
lingrad.addColorStop(0, '#00FF00');
lingrad.addColorStop(0.33, '#FF0000');
lingrad.addColorStop(0.66, '#0000FF');
lingrad.addColorStop(1, '#00FFFF');
ctx.fillStyle = lingrad;
ctx.strokeStyle = lingrad;
ctx.lineWidth = 10;
ctx.globalAlpha = 0.4;
ctx.strokeRect(5,5,190,190);
ctx.fillRect(0,0,50,50);
ctx.globalAlpha = 0.6;
ctx.strokeRect(35,35,130,130);
ctx.fillRect(50,50,50,50);
ctx.globalAlpha = 0.8;
ctx.strokeRect(65,65,70,70);
ctx.fillRect(100,100,50,50);
ctx.globalAlpha = 0.95;
ctx.fillRect(150,150,50,50);
};

tests['createLinearGradient() and transforms'] = function(ctx){
var lingrad = ctx.createLinearGradient(0,-100,0,100);
lingrad.addColorStop(0, '#00FF00');
lingrad.addColorStop(0.33, '#FF0000');
lingrad.addColorStop(0.66, '#0000FF');
lingrad.addColorStop(1, '#00FFFF');
ctx.fillStyle = lingrad;
ctx.translate(100, 100);
ctx.beginPath();
ctx.moveTo(-100, -100);
ctx.lineTo(100, -100);
ctx.lineTo(100, 100);
ctx.lineTo(-100, 100);
ctx.closePath();
ctx.globalAlpha = 0.5;
ctx.rotate(1.570795);
ctx.scale(0.6, 0.6);
ctx.fill();
};

tests['createRadialGradient()'] = function(ctx){
Expand Down Expand Up @@ -430,6 +512,17 @@ tests['globalAlpha'] = function(ctx){
ctx.globalAlpha = 0.5;
ctx.fillStyle = 'rgba(0,0,0,0.5)';
ctx.strokeRect(0,0,50,50);
ctx.fillRect(70,0,50,50);

ctx.globalAlpha = 0.25;
ctx.fillStyle = 'rgba(0,0,0,1)';
ctx.fillRect(70,70,50,50);

ctx.globalAlpha = 1;
ctx.fillStyle = 'rgba(0,0,0,0.25)';
ctx.fillRect(70,140,50,50);



ctx.globalAlpha = 0.8;
ctx.fillRect(20,20,20,20);
Expand Down