Skip to content

Commit

Permalink
Review users of cairo_rectangle_int_t for incorrect unsigned promotion.
Browse files Browse the repository at this point in the history 
Adrian Johnson discovered cases where we mistakenly compared the result
of unsigned arithmetic where we need signed quantities. Look for similar
cases in the users of cairo_rectangle_int_t.
  • Loading branch information
Chris Wilson committed Oct 30, 2008
1 parent 0e41561 commit 4b29988
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 148 deletions.
48 changes: 30 additions & 18 deletions src/cairo-analysis-surface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ _cairo_analysis_surface_intersect_clip_path (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
double x1, y1, x2, y2;
cairo_rectangle_int_t extent;
cairo_bool_t is_empty;

if (path == NULL) {
surface->current_clip.x = 0;
Expand All @@ -303,7 +304,7 @@ _cairo_analysis_surface_intersect_clip_path (void *abstract_surface,
extent.width = ceil (x2) - extent.x;
extent.height = ceil (y2) - extent.y;

_cairo_rectangle_intersect (&surface->current_clip, &extent);
is_empty = _cairo_rectangle_intersect (&surface->current_clip, &extent);
}

return CAIRO_STATUS_SUCCESS;
Expand All @@ -326,6 +327,7 @@ _cairo_analysis_surface_paint (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_status_t status, backend_status;
cairo_rectangle_int_t extents;
cairo_bool_t is_empty;

if (!surface->target->backend->paint)
backend_status = CAIRO_INT_STATUS_UNSUPPORTED;
Expand All @@ -342,14 +344,15 @@ _cairo_analysis_surface_paint (void *abstract_surface,

if (_cairo_operator_bounded_by_source (op)) {
cairo_rectangle_int_t source_extents;

status = _cairo_pattern_get_extents (source, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

status = _add_operation (surface, &extents, backend_status);

Expand All @@ -365,6 +368,7 @@ _cairo_analysis_surface_mask (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_int_status_t status, backend_status;
cairo_rectangle_int_t extents;
cairo_bool_t is_empty;

if (!surface->target->backend->mask)
backend_status = CAIRO_INT_STATUS_UNSUPPORTED;
Expand Down Expand Up @@ -411,16 +415,16 @@ _cairo_analysis_surface_mask (void *abstract_surface,
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);

status = _cairo_pattern_get_extents (mask, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

status = _add_operation (surface, &extents, backend_status);

Expand All @@ -441,7 +445,8 @@ _cairo_analysis_surface_stroke (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_status_t status, backend_status;
cairo_traps_t traps;
cairo_rectangle_int_t extents;
cairo_rectangle_int_t extents;
cairo_bool_t is_empty;

if (!surface->target->backend->stroke)
backend_status = CAIRO_INT_STATUS_UNSUPPORTED;
Expand All @@ -460,14 +465,15 @@ _cairo_analysis_surface_stroke (void *abstract_surface,

if (_cairo_operator_bounded_by_source (op)) {
cairo_rectangle_int_t source_extents;

status = _cairo_pattern_get_extents (source, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

if (_cairo_operator_bounded_by_mask (op)) {
cairo_box_t box;
Expand Down Expand Up @@ -509,7 +515,8 @@ _cairo_analysis_surface_fill (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_status_t status, backend_status;
cairo_traps_t traps;
cairo_rectangle_int_t extents;
cairo_rectangle_int_t extents;
cairo_bool_t is_empty;

if (!surface->target->backend->fill)
backend_status = CAIRO_INT_STATUS_UNSUPPORTED;
Expand All @@ -527,14 +534,15 @@ _cairo_analysis_surface_fill (void *abstract_surface,

if (_cairo_operator_bounded_by_source (op)) {
cairo_rectangle_int_t source_extents;

status = _cairo_pattern_get_extents (source, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

if (_cairo_operator_bounded_by_mask (op)) {
cairo_box_t box;
Expand Down Expand Up @@ -575,6 +583,7 @@ _cairo_analysis_surface_show_glyphs (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_status_t status, backend_status;
cairo_rectangle_int_t extents, glyph_extents;
cairo_bool_t is_empty;

/* Adapted from _cairo_surface_show_glyphs */
if (surface->target->backend->show_glyphs)
Expand Down Expand Up @@ -603,14 +612,15 @@ _cairo_analysis_surface_show_glyphs (void *abstract_surface,

if (_cairo_operator_bounded_by_source (op)) {
cairo_rectangle_int_t source_extents;

status = _cairo_pattern_get_extents (source, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

if (_cairo_operator_bounded_by_mask (op)) {
status = _cairo_scaled_font_glyph_device_extents (scaled_font,
Expand All @@ -620,7 +630,7 @@ _cairo_analysis_surface_show_glyphs (void *abstract_surface,
if (status)
return status;

_cairo_rectangle_intersect (&extents, &glyph_extents);
is_empty = _cairo_rectangle_intersect (&extents, &glyph_extents);
}

status = _add_operation (surface, &extents, backend_status);
Expand Down Expand Up @@ -652,6 +662,7 @@ _cairo_analysis_surface_show_text_glyphs (void *abstract_surface,
cairo_analysis_surface_t *surface = abstract_surface;
cairo_status_t status, backend_status;
cairo_rectangle_int_t extents, glyph_extents;
cairo_bool_t is_empty;

/* Adapted from _cairo_surface_show_glyphs */
backend_status = CAIRO_INT_STATUS_UNSUPPORTED;
Expand Down Expand Up @@ -684,14 +695,15 @@ _cairo_analysis_surface_show_text_glyphs (void *abstract_surface,

if (_cairo_operator_bounded_by_source (op)) {
cairo_rectangle_int_t source_extents;

status = _cairo_pattern_get_extents (source, &source_extents);
if (status)
return status;

_cairo_rectangle_intersect (&extents, &source_extents);
is_empty = _cairo_rectangle_intersect (&extents, &source_extents);
}

_cairo_rectangle_intersect (&extents, &surface->current_clip);
is_empty = _cairo_rectangle_intersect (&extents, &surface->current_clip);

if (_cairo_operator_bounded_by_mask (op)) {
status = _cairo_scaled_font_glyph_device_extents (scaled_font,
Expand All @@ -701,7 +713,7 @@ _cairo_analysis_surface_show_text_glyphs (void *abstract_surface,
if (status)
return status;

_cairo_rectangle_intersect (&extents, &glyph_extents);
is_empty = _cairo_rectangle_intersect (&extents, &glyph_extents);
}

status = _add_operation (surface, &extents, backend_status);
Expand Down
43 changes: 22 additions & 21 deletions src/cairo-clip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ _cairo_clip_path_intersect_to_rectangle (cairo_clip_path_t *clip_path,
_cairo_traps_fini (&traps);

_cairo_box_round_to_rectangle (&extents, &extents_rect);
_cairo_rectangle_intersect (rectangle, &extents_rect);
if (! _cairo_rectangle_intersect (rectangle, &extents_rect))
return CAIRO_STATUS_SUCCESS;

clip_path = clip_path->prev;
}
Expand All @@ -178,6 +179,9 @@ cairo_status_t
_cairo_clip_intersect_to_rectangle (cairo_clip_t *clip,
cairo_rectangle_int_t *rectangle)
{
cairo_status_t status;
cairo_bool_t is_empty;

if (!clip)
return CAIRO_STATUS_SUCCESS;

Expand All @@ -187,16 +191,13 @@ _cairo_clip_intersect_to_rectangle (cairo_clip_t *clip,
}

if (clip->path) {
cairo_status_t status;

status = _cairo_clip_path_intersect_to_rectangle (clip->path,
rectangle);
if (status)
return status;
}

if (clip->has_region) {
cairo_status_t status = CAIRO_STATUS_SUCCESS;
cairo_region_t intersection;

_cairo_region_init_rect (&intersection, rectangle);
Expand All @@ -214,7 +215,7 @@ _cairo_clip_intersect_to_rectangle (cairo_clip_t *clip,
}

if (clip->surface)
_cairo_rectangle_intersect (rectangle, &clip->surface_rect);
is_empty = _cairo_rectangle_intersect (rectangle, &clip->surface_rect);

return CAIRO_STATUS_SUCCESS;
}
Expand Down Expand Up @@ -421,7 +422,7 @@ _cairo_clip_intersect_mask (cairo_clip_t *clip,
cairo_pattern_union_t pattern;
cairo_box_t extents;
cairo_rectangle_int_t surface_rect, target_rect;
cairo_surface_t *surface;
cairo_surface_t *surface = NULL;
cairo_status_t status;

if (clip->all_clipped)
Expand All @@ -434,22 +435,21 @@ _cairo_clip_intersect_mask (cairo_clip_t *clip,
_cairo_traps_extents (traps, &extents);
_cairo_box_round_to_rectangle (&extents, &surface_rect);

if (clip->surface != NULL)
_cairo_rectangle_intersect (&surface_rect, &clip->surface_rect);
if (clip->surface != NULL) {
if (! _cairo_rectangle_intersect (&surface_rect, &clip->surface_rect))
goto DONE;
}

/* Intersect with the target surface rectangle so we don't use
* more memory and time than we need to. */
status = _cairo_surface_get_extents (target, &target_rect);
if (status == CAIRO_STATUS_SUCCESS)
_cairo_rectangle_intersect (&surface_rect, &target_rect);
if (status == CAIRO_STATUS_SUCCESS) {
if (! _cairo_rectangle_intersect (&surface_rect, &target_rect))
goto DONE;
}

if (surface_rect.width == 0 || surface_rect.height == 0) {
surface = NULL;
status = CAIRO_STATUS_SUCCESS;
if (clip->surface != NULL)
cairo_surface_destroy (clip->surface);
if (surface_rect.width == 0 || surface_rect.height == 0)
goto DONE;
}

_cairo_pattern_init_solid (&pattern.solid, CAIRO_COLOR_WHITE,
CAIRO_CONTENT_COLOR);
Expand Down Expand Up @@ -539,11 +539,10 @@ _cairo_clip_intersect_mask (cairo_clip_t *clip,
cairo_surface_destroy (surface);
return status;
}

cairo_surface_destroy (clip->surface);
}

DONE:
cairo_surface_destroy (clip->surface);
clip->surface = surface;
clip->surface_rect = surface_rect;
clip->serial = _cairo_surface_allocate_clip_serial (target);
Expand Down Expand Up @@ -740,10 +739,12 @@ _cairo_clip_int_rect_to_user (cairo_gstate_t *gstate,

double x1 = clip_rect->x;
double y1 = clip_rect->y;
double x2 = clip_rect->x + clip_rect->width;
double y2 = clip_rect->y + clip_rect->height;
double x2 = clip_rect->x + (int) clip_rect->width;
double y2 = clip_rect->y + (int) clip_rect->height;

_cairo_gstate_backend_to_user_rectangle (gstate, &x1, &y1, &x2, &y2, &is_tight);
_cairo_gstate_backend_to_user_rectangle (gstate,
&x1, &y1, &x2, &y2,
&is_tight);

user_rect->x = x1;
user_rect->y = y1;
Expand Down
4 changes: 2 additions & 2 deletions src/cairo-gstate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1785,8 +1785,8 @@ _cairo_gstate_transform_glyphs_to_backend (cairo_gstate_t *gstate,
*/
x1 = surface_extents.x - 2*scale;
y1 = surface_extents.y - 2*scale;
x2 = surface_extents.x + surface_extents.width + scale;
y2 = surface_extents.y + surface_extents.height + scale;
x2 = surface_extents.x + (int) surface_extents.width + scale;
y2 = surface_extents.y + (int) surface_extents.height + scale;
}

if (!drop)
Expand Down
13 changes: 8 additions & 5 deletions src/cairo-pattern.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,7 @@ _cairo_pattern_acquire_surface_for_surface (cairo_surface_pattern_t *pattern,
attr->acquired = TRUE;
} else {
cairo_rectangle_int_t extents;
cairo_bool_t is_empty;

status = _cairo_surface_get_extents (pattern->surface, &extents);
if (status)
Expand All @@ -1932,8 +1933,8 @@ _cairo_pattern_acquire_surface_for_surface (cairo_surface_pattern_t *pattern,
} else {
double x1 = x;
double y1 = y;
double x2 = x + width;
double y2 = y + height;
double x2 = x + (int) width;
double y2 = y + (int) height;

_cairo_matrix_transform_bounding_box (&attr->matrix,
&x1, &y1, &x2, &y2,
Expand All @@ -1950,9 +1951,11 @@ _cairo_pattern_acquire_surface_for_surface (cairo_surface_pattern_t *pattern,
sampled_area.y += ty;

/* Never acquire a larger area than the source itself */
_cairo_rectangle_intersect (&extents, &sampled_area);
is_empty = _cairo_rectangle_intersect (&extents, &sampled_area);
}

/* XXX can we use is_empty? */

status = _cairo_surface_clone_similar (dst, pattern->surface,
extents.x, extents.y,
extents.width, extents.height,
Expand Down Expand Up @@ -2237,8 +2240,8 @@ _cairo_pattern_get_extents (cairo_pattern_t *pattern,
_cairo_pattern_analyze_filter (surface_pattern, &pad);
x1 = surface_extents.x - pad;
y1 = surface_extents.y - pad;
x2 = surface_extents.x + surface_extents.width + pad;
y2 = surface_extents.y + surface_extents.height + pad;
x2 = surface_extents.x + (int) surface_extents.width + pad;
y2 = surface_extents.y + (int) surface_extents.height + pad;

imatrix = pattern->matrix;
status = cairo_matrix_invert (&imatrix);
Expand Down
6 changes: 5 additions & 1 deletion src/cairo-rectangle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ _cairo_box_round_to_rectangle (const cairo_box_t *box,
rectangle->height = _cairo_fixed_integer_ceil (box->p2.y) - rectangle->y;
}

void
cairo_bool_t
_cairo_rectangle_intersect (cairo_rectangle_int_t *dst,
const cairo_rectangle_int_t *src)
{
Expand All @@ -114,11 +114,15 @@ _cairo_rectangle_intersect (cairo_rectangle_int_t *dst,
dst->y = 0;
dst->width = 0;
dst->height = 0;

return FALSE;
} else {
dst->x = x1;
dst->y = y1;
dst->width = x2 - x1;
dst->height = y2 - y1;

return TRUE;
}
}

Expand Down
Loading

0 comments on commit 4b29988

Please sign in to comment.