Last Comment Bug 562746 - Update cairo to 1.10
: Update cairo to 1.10
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal with 4 votes (vote)
: mozilla7
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
: 526977 554142 (view as bug list)
Depends on: 456448 663258 668549 689962 691167 698007 700003 716462 733714 768362 801794 808249 873313 877492 896537 974310 616700 638594 656465 659676 660448 671064 pixman-coord 672726 689217 692350 720035 725937 730571 989592
Blocks: 567370 629012 404637 526977 545632 563255 566283 601512 614721 646611
  Show dependency treegraph
 
Reported: 2010-04-29 13:39 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2014-07-20 08:14 PDT (History)
43 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
First try (1.21 MB, patch)
2010-05-05 10:50 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
v2 (1.28 MB, patch)
2010-05-06 19:44 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
v3 (1.29 MB, patch)
2010-05-26 08:15 PDT, Jeff Muizelaar [:jrmuizel]
timeless: feedback-
timeless: feedback-
timeless: feedback-
Details | Diff | Splinter Review
Additional patch to fix qt port compilation (4.94 KB, patch)
2010-06-03 05:10 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
v4 - timeless' concerns not yet addressed (1.96 MB, patch)
2010-10-22 10:16 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
v5 (1.96 MB, patch)
2010-11-16 13:33 PST, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Updated to m-c 716b7303beea (1.95 MB, patch)
2011-03-03 02:37 PST, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Cairo Qt fix (937 bytes, patch)
2011-03-03 16:51 PST, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Cairo Qt fix v2 (966 bytes, patch)
2011-03-03 17:07 PST, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
rebased on top of 4e771e65764a (1.77 MB, patch)
2011-03-14 12:52 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
rebased on top of 4e771e65764a v2 (1.97 MB, patch)
2011-03-15 08:34 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
rebased on top of 4f07bebb993b (1.97 MB, patch)
2011-03-16 12:08 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
rebased on top of 4f07bebb993b v2 (1.98 MB, patch)
2011-03-23 08:49 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
rebased again (2.03 MB, patch)
2011-05-10 07:04 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Rebased (for real this time) (2.03 MB, patch)
2011-05-10 11:58 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Rendering bug testcase (394 bytes, text/html)
2011-05-11 12:21 PDT, :Ehsan Akhgari
no flags Details
v2 - fixes clipping problems on win32 and perf problems on linux (2.04 MB, patch)
2011-05-11 15:46 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Remove use of EXTEND_NONE on quartz (1.06 KB, patch)
2011-05-13 07:06 PDT, Jeff Muizelaar [:jrmuizel]
roc: review+
Details | Diff | Splinter Review
Add reftest that fails with the quartz backend from cairo 1.10 (1.33 KB, patch)
2011-05-13 08:15 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Add reftest that fails with the quartz backend from cairo 1.10 (1.91 KB, patch)
2011-05-13 08:15 PDT, Jeff Muizelaar [:jrmuizel]
ehsan: review+
Details | Diff | Splinter Review
rebased on top 1f3777d4ed8b (2.03 MB, patch)
2011-05-18 12:51 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Fixes some of the android reftest failures (2.04 MB, patch)
2011-05-20 06:41 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
rebased again (2.04 MB, patch)
2011-05-24 12:56 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Delete some code we don't need (19.89 KB, patch)
2011-06-01 11:58 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review
Inline the check method differently (1.43 KB, patch)
2011-06-01 15:47 PDT, Jeff Muizelaar [:jrmuizel]
no flags Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2010-04-29 13:39:24 PDT
This update should get us up-to-date with last cairo trunk
Comment 1 Jeff Muizelaar [:jrmuizel] 2010-05-05 10:50:59 PDT
Created attachment 443664 [details] [diff] [review]
First try
Comment 2 Jeff Muizelaar [:jrmuizel] 2010-05-06 13:12:45 PDT
This gives us two failures on OS X:
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/smil/sort/sort-additive-1.svg |
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/text-font-weight-01.svg |
Comment 3 Jeff Muizelaar [:jrmuizel] 2010-05-06 19:44:26 PDT
Created attachment 444016 [details] [diff] [review]
v2

Fixes some d2d compilation problems and updates to cairo trunk
Comment 4 Jeff Muizelaar [:jrmuizel] 2010-05-26 08:15:23 PDT
Created attachment 447523 [details] [diff] [review]
v3

More updates. Doesn't build on win32 for some reason.
Comment 5 Oleg Romashin (:romaxa) 2010-06-03 05:10:26 PDT
Created attachment 448992 [details] [diff] [review]
Additional patch to fix qt port compilation
Comment 6 Robert Longson 2010-06-03 06:32:27 PDT
(In reply to comment #2)
> This gives us two failures on OS X:
> REFTEST TEST-UNEXPECTED-FAIL |
> file:///builds/slave/sendchange-macosx-unittest/mozilla/layout/reftests/svg/smil/sort/sort-additive-1.svg
> |

Are you sure this one isn't just random orange i.e. bug 547801
Comment 7 Jeff Muizelaar [:jrmuizel] 2010-06-15 12:07:26 PDT
I'm seeing some new weird failures on OS X. I get aliased fonts for some things on tests like:

layout/reftests/bugs/179596-2.html
Comment 8 Oleg Romashin (:romaxa) 2010-06-15 12:32:50 PDT
 > layout/reftests/bugs/179596-2.html

Can we just push new cairo update, and record all known issues as bugs ?
Comment 9 Jeff Muizelaar [:jrmuizel] 2010-06-15 12:36:41 PDT
Not really, it will cause a bunch of orange. Help fixing the issues would be great though :)
Comment 10 Jeff Muizelaar [:jrmuizel] 2010-07-28 20:54:21 PDT
(In reply to comment #7)
> I'm seeing some new weird failures on OS X. I get aliased fonts for some things
> on tests like:
> 
> layout/reftests/bugs/179596-2.html

Looks like this is a bug with the state management of CGContextSetShouldAntialias
Comment 11 Ginn Chen 2010-08-18 03:58:51 PDT
*** Bug 526977 has been marked as a duplicate of this bug. ***
Comment 12 timeless 2010-10-12 10:59:05 PDT
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t

>+static cairo_status_t
>+_cairo_image_surface_fixup_unbounded (cairo_image_surface_t *dst,
>+				      const cairo_composite_rectangles_t *rects,
>+				      cairo_clip_t *clip)
>+{
>+    pixman_image_t *mask = NULL;
>+    pixman_box32_t boxes[4];
>+    int i, mask_x = 0, mask_y = 0, n_boxes = 0;
>+
>+    if (clip != NULL) {
>+	cairo_surface_t *clip_surface;
>+	int clip_x, clip_y;
>+
>+	clip_surface = _cairo_clip_get_surface (clip, &dst->base, &clip_x, &clip_y);
>+	if (unlikely (clip_surface->status))
>+	    return clip_surface->status;
>+
>+	mask = ((cairo_image_surface_t *) clip_surface)->pixman_image;
>+	mask_x = -clip_x;
>+	mask_y = -clip_y;
>+    } else {

Coverity complains that this leaks clip_surface when clip_surface->status is not a failure. Yet pixman_image is rescued, but its container is leaked.
Comment 13 timeless 2010-10-12 12:09:16 PDT
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t


>+static cairo_status_t
>+_composite_boxes (cairo_image_surface_t *dst,
>+		  cairo_operator_t op,
>+		  const cairo_pattern_t *pattern,
>+		  cairo_boxes_t *boxes,
>+		  cairo_antialias_t antialias,
>+		  cairo_clip_t *clip,
>+		  const cairo_composite_rectangles_t *extents)
>+{
>+    cairo_region_t *clip_region = NULL;
>+    cairo_bool_t need_clip_mask = FALSE;
>+    cairo_status_t status;
>+    struct _cairo_boxes_chunk *chunk;
>+    uint32_t pixel;
>+    int i;
>+
>+    if (clip != NULL) {
>+	status = _cairo_clip_get_region (clip, &clip_region);
>+	need_clip_mask = status == CAIRO_INT_STATUS_UNSUPPORTED;
>+	if (need_clip_mask &&
>+	    (op == CAIRO_OPERATOR_SOURCE || ! extents->is_bounded))
>+	{
...
>+	}
>+
>+	if (clip_region != NULL && cairo_region_num_rectangles (clip_region) == 1)
>+	    clip_region = NULL;
>+    }
>+
>+    if (antialias != CAIRO_ANTIALIAS_NONE) {
...
>+    }
>+
>+    status = CAIRO_STATUS_SUCCESS;
>+    if (! need_clip_mask &&
>+	pattern_to_pixel ((cairo_solid_pattern_t *) pattern, op, dst->pixman_format,
>+			  &pixel))
>+    {
...
>+    }
>+    else
>+    {
mask is initialized to NULL:
>+	pixman_image_t *src = NULL, *mask = NULL;
...

let need_clip_mask be false:
>+	if (need_clip_mask) {
...
>+	}
>+

let pattern be false:
>+	if (pattern != NULL) {
...
>+	} else {

src is now null because mask was null:
>+	    src = mask;
...
>+	    mask = NULL;
>+	}
>+
>+	for (chunk = &boxes->chunks; chunk != NULL; chunk = chunk->next) {
>+	    const cairo_box_t *box = chunk->base;
>+
>+	    for (i = 0; i < chunk->count; i++) {
>+		int x1 = _cairo_fixed_integer_round (box[i].p1.x);
>+		int y1 = _cairo_fixed_integer_round (box[i].p1.y);
>+		int x2 = _cairo_fixed_integer_round (box[i].p2.x);
>+		int y2 = _cairo_fixed_integer_round (box[i].p2.y);
>+
>+		if (x2 == x1 || y2 == y1)
>+		    continue;

here src and mask are passed as null:
>+		pixman_image_composite32 (pixman_op,
>+                                          src, mask, dst->pixman_image,
>+                                          x1 + src_x,  y1 + src_y,
>+                                          x1 + mask_x, y1 + mask_y,
>+                                          x1, y1,
>+                                          x2 - x1, y2 - y1);

And we crash under _pixman_image_validate (src);
Comment 14 timeless 2010-10-12 12:13:09 PDT
Comment on attachment 447523 [details] [diff] [review]
v3

>diff --git a/gfx/cairo/cairo/src/cairo-image-surface.c b/gfx/cairo/cairo/src/cairo-image-surface.c
>--- a/gfx/cairo/cairo/src/cairo-image-surface.c
>+++ b/gfx/cairo/cairo/src/cairo-image-surface.c
>@@ -1073,22 +823,3228 @@ static cairo_status_t

+static cairo_status_t
+_clip_and_composite (cairo_image_surface_t	*dst,
+		     cairo_operator_t		 op,
+		     const cairo_pattern_t	*src,
+		     image_draw_func_t		 draw_func,
+		     void			*draw_closure,
+		     cairo_composite_rectangles_t*extents,
+		     cairo_clip_t		*clip)
+{
+    cairo_status_t status;
+    cairo_region_t *clip_region = NULL;
+    cairo_bool_t need_clip_surface = FALSE;
+
let clip be null:
+    if (clip != NULL) {
...
+    }
+
let clip_region be null:
+    if (clip_region != NULL) {
...
+    }
+
let this function return false:
+    if (reduce_alpha_op (dst, op, src)) {
...
+    }
+
let op be CAIRO_OPERATOR_SOURCE:
+    if (op == CAIRO_OPERATOR_SOURCE) {
clip is null and we crash in here:
+	status = _clip_and_composite_source (clip, src,
+					     draw_func, draw_closure,
+					     dst, &extents->bounded);
Comment 15 Vladimir Vukicevic [:vlad] [:vladv] 2010-10-17 20:22:51 PDT
Can you also, as part of this, normalize line endings in the d2d code?  That is, remove all the dos line endings...
Comment 16 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:12:07 PDT
(In reply to comment #15)
> Can you also, as part of this, normalize line endings in the d2d code?  That
> is, remove all the dos line endings...

I'll do that separately.
Comment 17 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:13:42 PDT
This seems to be causing intermittent failures of 

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/tryserver_fedora64-debug_test-reftest/build/reftest/tests/layout/reftests/svg/smil/sort/sort-additive-1.svg |

The failures seem clipping unrelated and perhaps not the same as bug 547801
Comment 18 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:16:18 PDT
Created attachment 485329 [details] [diff] [review]
v4 - timeless' concerns not yet addressed
Comment 19 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:21:04 PDT
> >+
> >+	clip_surface = _cairo_clip_get_surface (clip, &dst->base, &clip_x, &clip_y);
> >+	if (unlikely (clip_surface->status))
> >+	    return clip_surface->status;
> >+
> >+	mask = ((cairo_image_surface_t *) clip_surface)->pixman_image;
> >+	mask_x = -clip_x;
> >+	mask_y = -clip_y;
> >+    } else {
> 
> Coverity complains that this leaks clip_surface when clip_surface->status is
> not a failure. Yet pixman_image is rescued, but its container is leaked.

The surface is owned by the clip_path and destroyed in _cairo_clip_path_destroy
Comment 20 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:27:23 PDT
(In reply to comment #13)
> Comment on attachment 447523 [details] [diff] [review]
> v3

> 
> let pattern be false:

I'm don't think there are cases where pattern is NULL
Comment 21 Jeff Muizelaar [:jrmuizel] 2010-10-22 10:30:47 PDT
(In reply to comment #14)
> Comment on attachment 447523 [details] [diff] [review]
> v3

> let op be CAIRO_OPERATOR_SOURCE:
> +    if (op == CAIRO_OPERATOR_SOURCE) {
> clip is null and we crash in here:
> +    status = _clip_and_composite_source (clip, src,
> +                         draw_func, draw_closure,
> +                         dst, &extents->bounded);

_clip_and_composite_source seems to deal with a NULL clip, so I'm not sure why we'd crash.
Comment 22 Jeff Muizelaar [:jrmuizel] 2010-10-22 14:02:16 PDT
Karl,

Any guesses why we could get this kind of rendering: http://pastebin.mozilla.org/824349 ?
Comment 23 Karl Tomlinson (:karlt) 2010-10-25 00:25:16 PDT
No ideas, sorry.
Is the assumption that the similar failure in comment 2 was bug 547801, so the new issue is fedora 64 specific?  or intermittent?
Comment 24 Jeff Muizelaar [:jrmuizel] 2010-11-02 16:30:38 PDT
Looks like it was caused by:

commit 6b77567b6ef28710c7707ab82c7fa95c810152d1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jan 22 15:54:45 2010 +0000

    path: Compute coarse bounds upon construction.
    
    Frequently we only need the coarse path bounds, so avoid walking over
    the list of points once more as we can cheaply track the extents during
    construction.
Comment 25 Jeff Muizelaar [:jrmuizel] 2010-11-05 00:39:25 PDT
and should be fixed with:

diff --git a/src/cairo-path-bounds.c b/src/cairo-path-bounds.c
index 8f9bdc3..d46b8e5 100644
--- a/src/cairo-path-bounds.c
+++ b/src/cairo-path-bounds.c
@@ -331,7 +331,7 @@ _cairo_path_fixed_extents (const cairo_path_fixed_t *path,
 
     if (! path->has_curve_to) {
 	*box = path->extents;
-	return path->extents.p1.x < path->extents.p2.x;
+	return path->extents.p1.x <= path->extents.p2.x;
     }
 
     _cairo_path_bounder_init (&bounder);
Comment 26 Jeff Muizelaar [:jrmuizel] 2010-11-08 16:33:25 PST
It seems like 3b1c0a4bd66660780095e6016e3db451f34503a3 is the other commit that broke a win32 test.
Comment 27 Jeff Muizelaar [:jrmuizel] 2010-11-15 14:28:38 PST
This could be a lot of work to fix...
Comment 28 Jeff Muizelaar [:jrmuizel] 2010-11-16 13:33:12 PST
Created attachment 490975 [details] [diff] [review]
v5
Comment 29 Joe Drew (not getting mail) 2010-12-09 13:08:42 PST
Right now I don't think we absolutely require Cairo 1.10. If it becomes a hard blocker for anyone, this could change.
Comment 30 Uli Link (:ul-mcamafia) 2010-12-19 08:21:22 PST
(In reply to comment #29)
> Right now I don't think we absolutely require Cairo 1.10. If it becomes a hard
> blocker for anyone, this could change.

requiring cairo 1.10 will require update libpixman-1 (which is easy) and also require updating glib-2.0 for CentOS5/RHEL5/ScienticLinux.
Building cairo 1.10 requires at least Glib 2.14.

Dropping support for a widespread used Enterprise Linux distro isn't a good idea or at least Firefox 3.6 should get security fixes for 3 more years until RHEL5 is EoL ;-)
Comment 31 Oleg Romashin (:romaxa) 2010-12-23 11:23:39 PST
on www.ask.com Fennec show partially black  screen with cairo 1.10
Comment 32 christian 2011-01-25 09:28:20 PST
Bug 601512 is a hardblocker that says it will be fixed by this bug. I guess this needs to hardblock as well, correct?
Comment 33 Cameron Kaiser [:spectre] 2011-02-05 11:23:36 PST
Would this also repair bug 595671, based on comments in that bug?
Comment 34 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 01:15:17 PST
Applying to m-c 716b7303beea gives

applying imp-562746-cairo-1.10
patching file gfx/cairo/cairo/src/Makefile.in
Hunk #3 FAILED at 270
1 out of 3 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/Makefile.in.rej
patching file gfx/cairo/cairo/src/cairo-d2d-private.h
Hunk #1 FAILED at 37
1 out of 1 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-d2d-private.h.rej
patching file gfx/cairo/cairo/src/cairo-d2d-surface.cpp
Hunk #1 FAILED at 33
Hunk #2 FAILED at 624
Hunk #3 FAILED at 710
Hunk #4 FAILED at 1388
Hunk #5 FAILED at 1435
Hunk #6 FAILED at 1467
Hunk #7 FAILED at 1572
Hunk #8 FAILED at 2251
Hunk #9 FAILED at 2367
Hunk #10 FAILED at 2440
Hunk #11 FAILED at 2512
Hunk #12 FAILED at 3120
Hunk #13 FAILED at 3212
Hunk #14 FAILED at 3391
Hunk #15 FAILED at 3447
Hunk #16 FAILED at 3558
Hunk #17 FAILED at 3587
Hunk #18 FAILED at 3672
Hunk #19 FAILED at 3727
Hunk #20 FAILED at 3773
Hunk #21 FAILED at 3796
Hunk #22 FAILED at 3856
22 out of 22 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-d2d-surface.cpp.rej
patching file gfx/cairo/cairo/src/cairo-gstate.c
Hunk #22 FAILED at 1956
1 out of 25 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-gstate.c.rej
patching file gfx/cairo/cairo/src/cairo-qt-surface.cpp
Hunk #7 FAILED at 1601
1 out of 8 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-qt-surface.cpp.rej
patching file gfx/cairo/cairo/src/cairo-quartz-surface.c
Hunk #4 FAILED at 117
Hunk #5 FAILED at 149
Hunk #20 FAILED at 1762
Hunk #27 FAILED at 2402
Hunk #30 FAILED at 2855
5 out of 35 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-quartz-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-surface-private.h
Hunk #2 FAILED at 35
1 out of 2 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-surface-private.h.rej
patching file gfx/cairo/cairo/src/cairo-surface.c
Hunk #2 FAILED at 34
Hunk #5 FAILED at 208
2 out of 37 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-win32-surface.c
Hunk #4 FAILED at 116
1 out of 12 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-win32-surface.c.rej
patching file gfx/cairo/cairo/src/cairo-xlib-surface.c
Hunk #49 FAILED at 3820
Hunk #50 FAILED at 3845
Hunk #51 FAILED at 3914
Hunk #66 succeeded at 4682 with fuzz 1 (offset 10 lines).
Hunk #68 FAILED at 4734
4 out of 70 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo-xlib-surface.c.rej
patching file gfx/cairo/cairo/src/cairo.h
Hunk #6 FAILED at 2065
Hunk #7 FAILED at 2114
Hunk #9 FAILED at 2216
3 out of 14 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairo.h.rej
patching file gfx/cairo/cairo/src/cairoint.h
Hunk #29 FAILED at 2477
1 out of 30 hunks FAILED -- saving rejects to file gfx/cairo/cairo/src/cairoint.h.rej
patching file gfx/thebes/gfxASurface.cpp
Hunk #1 FAILED at 475
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxASurface.cpp.rej
patching file gfx/thebes/gfxASurface.h
Hunk #1 FAILED at 90
1 out of 1 hunks FAILED -- saving rejects to file gfx/thebes/gfxASurface.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh imp-562746-cairo-1.10

I'll try to update what I can to test bug 637828.
Comment 35 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 02:37:56 PST
Created attachment 516549 [details] [diff] [review]
Updated to m-c 716b7303beea

Thankfully Timothy had a fairly recent rebase that made this much easier.

Fixes bug 637828.
Comment 36 Benjamin Stover (:stechz) 2011-03-03 10:59:34 PST
This makes some graphics corruptions issues on the start page worse, but fixes a lot of our other bugs. We'll need to see what's going on there. I think Fennec does need this for better or for worse.
Comment 37 Brad Lassey [:blassey] (use needinfo?) 2011-03-03 12:13:02 PST
to add to what stechz said, two bugs marked blocking-fennec are solved by this update, requesting blocking-fennec for this bug so it can be triaged
Comment 38 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 12:37:42 PST
The path of least resistance (although also least elegance) to using cairo 1.10(.2?) and pixman 0.21(.6?) for fennec would be
 - copy vanilla releases into gfx/cairo/new-cairo or something like that.
 - apply pixman-android-cpu-detect patch.  There might be others we need.
 - copy build goop from gfx/cairo/cairo/src to gfx/cairo/new-cairo/src
 - update for build goop for cairo+pixman changes
 - split off the gecko (non-cairo) parts of Jeff's patch here
 - add a new --enable-new-cairo configure flag that selects new-cairo

new-cairo can be built, linked, and shipped in libxul like the current in-tree cairo.  I would guess this is a full day's worth of work for someone familiar with the build system (e.g., me).

Part of the decision to take this is the value in fixing the dependent graphical bugs, which is mostly known already.  The other part of the decision is the perf difference with latest cairo+pixman.  We won't have an accurate estimate of perf until we do all the import+build work.  It would normally be possible to build --enable-system-cairo/--enable-system-pixman and package the .so's with fennec, but I imagine that porting both to android might be as much or more work than just importing both into our tree.
Comment 39 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 12:56:43 PST
I sent off a try run (a8e04b6d8135) with the patch here to get a rough idea of perf difference.  Very rough because the runs will still use older in-tree pixman and the update here to a not-latest cairo.
Comment 40 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 13:43:07 PST
(For future reference, this patch doesn't build on maemo-qt: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1299185767.1299187744.7888.gz .)
Comment 41 Oleg Romashin (:romaxa) 2011-03-03 13:57:45 PST
> reply to comment #40)
I guess you need https://bugzilla.mozilla.org/attachment.cgi?id=448992&action=edit(In
Comment 42 Brad Lassey [:blassey] (use needinfo?) 2011-03-03 14:52:43 PST
cjones, can you file a bug for taking that subset of patches? and leave this one for the full update.
Comment 43 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 14:53:33 PST
Yep, was just about to do that.
Comment 44 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 15:05:46 PST
Going to move the fennec deps here to bug 638594, apologies for bug spam.
Comment 45 Brad Lassey [:blassey] (use needinfo?) 2011-03-03 15:08:25 PST
removing the blocking flag from this and adding it to the bug that cjones just filed
Comment 46 Oleg Romashin (:romaxa) 2011-03-03 16:51:11 PST
Created attachment 516749 [details] [diff] [review]
Cairo Qt fix
Comment 47 Oleg Romashin (:romaxa) 2011-03-03 16:55:45 PST
Comment on attachment 516749 [details] [diff] [review]
Cairo Qt fix


oh, /gfx/thebes/gfxQPainterSurface.cpp - part is wrong
Comment 48 Oleg Romashin (:romaxa) 2011-03-03 17:07:01 PST
Created attachment 516750 [details] [diff] [review]
Cairo Qt fix v2

this compiles fine
Comment 49 Timothy Nikkel (:tnikkel) 2011-03-03 18:12:46 PST
Comment on attachment 516549 [details] [diff] [review]
Updated to m-c 716b7303beea

I'm not 100% confident in the rebasing I did, but it was enough to get it working and to test it on top of (what was then) current trunk. In case anyone was thinking of committing this somewhere...
Comment 50 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-03-03 20:35:45 PST
That's OK, I'm not 100% confident in my rebasing either :).  This was more for proof-of-concept purposes.
Comment 51 Jeff Muizelaar [:jrmuizel] 2011-03-14 12:52:05 PDT
Created attachment 519213 [details] [diff] [review]
rebased on top of 4e771e65764a
Comment 52 Markus Stange [:mstange] 2011-03-15 01:41:34 PDT
Comment on attachment 519213 [details] [diff] [review]
rebased on top of 4e771e65764a

The added files are missing in this patch.
Comment 53 Jeff Muizelaar [:jrmuizel] 2011-03-15 08:34:49 PDT
Created attachment 519411 [details] [diff] [review]
rebased on top of 4e771e65764a v2

This one is more likely to build.
Comment 54 Jeff Muizelaar [:jrmuizel] 2011-03-15 08:44:02 PDT
I'm seeing a reftest failure on OS X on test bugs/615121-1.html
Comment 55 Jeff Muizelaar [:jrmuizel] 2011-03-15 14:48:45 PDT
(In reply to comment #54)
> I'm seeing a reftest failure on OS X on test bugs/615121-1.html

This is caused by 9c0d761bfcdd28d52c83d74f46dd3c709ae0fa69 which should be pretty easy to fix.
Comment 56 Jeff Muizelaar [:jrmuizel] 2011-03-16 12:08:35 PDT
Created attachment 519737 [details] [diff] [review]
rebased on top of 4f07bebb993b
Comment 57 Jeff Muizelaar [:jrmuizel] 2011-03-23 08:49:49 PDT
Created attachment 521189 [details] [diff] [review]
rebased on top of 4f07bebb993b v2

This fixes the quartz issue and adds a buggy version of gdi changes.
Comment 58 Jeff Muizelaar [:jrmuizel] 2011-03-23 09:28:51 PDT
an alternative approach to fallback:
http://people.mozilla.com/~jmuizelaar/cairo-update-march
Comment 59 Jeff Muizelaar [:jrmuizel] 2011-04-26 12:00:31 PDT
http://people.mozilla.com/~jmuizelaar/cairo-update-march2
Comment 60 Jeff Muizelaar [:jrmuizel] 2011-04-26 14:16:43 PDT
Seems like there's a memory leak when running reftests with d2d.
Comment 61 Jeff Muizelaar [:jrmuizel] 2011-05-10 07:04:14 PDT
Created attachment 531318 [details] [diff] [review]
rebased again
Comment 62 :Ehsan Akhgari 2011-05-10 11:58:51 PDT
Created attachment 531400 [details] [diff] [review]
Rebased (for real this time)
Comment 63 :Ehsan Akhgari 2011-05-11 12:21:12 PDT
Created attachment 531709 [details]
Rendering bug testcase

I found this bug when testing Jeff's patch on Wikipedia.

Try zooming in on this test case.  A vertical line appears at the boundaries of the <a> elements in some zoom levels.
Comment 64 Jeff Muizelaar [:jrmuizel] 2011-05-11 15:46:45 PDT
Created attachment 531780 [details] [diff] [review]
v2 - fixes clipping problems on win32 and perf problems on linux

The known issues left are:
- d2d leak
- mac image drawing bug that ehsan posted about
Comment 65 Jeff Muizelaar [:jrmuizel] 2011-05-13 07:06:41 PDT
Created attachment 532207 [details] [diff] [review]
Remove use of EXTEND_NONE on quartz

The newer quartz backend actually supports extend none properly. We currently (and in the update) patch extend_pad to behave like the old extend_none instead of a real extend_pad, so this change shouldn't have any rendering differences now and fixes the reftest when we update.
Comment 66 Jeff Muizelaar [:jrmuizel] 2011-05-13 08:15:00 PDT
Created attachment 532241 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10
Comment 67 Jeff Muizelaar [:jrmuizel] 2011-05-13 08:15:50 PDT
Created attachment 532243 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10
Comment 68 :Ehsan Akhgari 2011-05-13 08:22:34 PDT
Comment on attachment 532243 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10

Please specify the line-height explicitly, because it may change because of any number of reasons.  Also, please make sure that the test works fine on Android before landing it (and if it doesn't, you can just tag it with fails-if).

r=me with that.
Comment 69 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-14 02:50:34 PDT
Comment on attachment 532207 [details] [diff] [review]
Remove use of EXTEND_NONE on quartz

Review of attachment 532207 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 70 Jeff Muizelaar [:jrmuizel] 2011-05-18 12:51:23 PDT
Created attachment 533382 [details] [diff] [review]
rebased on top 1f3777d4ed8b
Comment 71 Jeff Muizelaar [:jrmuizel] 2011-05-18 13:01:05 PDT
I just noticed that there are a bunch of android reftest failures caused by this. They seem to be largely caused by the difference between drawing things like: [q][uestions] and [questions]. I'm not sure why that's happening yet though.
Comment 72 Jeff Muizelaar [:jrmuizel] 2011-05-20 06:41:19 PDT
Created attachment 533949 [details] [diff] [review]
Fixes some of the android reftest failures

This fixes most of the android reftest failures. There are two left:
generated-content/display-types-01.html
counters/counter-reset-integer-range.html
Comment 73 Jeff Muizelaar [:jrmuizel] 2011-05-24 12:56:00 PDT
Created attachment 534865 [details] [diff] [review]
rebased again

Most things are good with this version but I intermittently get:

TEST-PASS | chrome://mochitests/content/browser/toolkit/components/console/hudservice/tests/browser/browser_webconsole_bug_613642_maintain_scroll.js | scroll location is not at the top
TEST-PASS | chrome://mochitests/content/browser/toolkit/components/console/hudservice/tests/browser/browser_webconsole_bug_613642_maintain_scroll.js | scroll location updated (moved to top)
###!!! ASSERTION: recursive painting not permitted: '!IsPainting()', file 

command timed out: 1200 seconds without output, killing pid 3440
Comment 74 Jeff Muizelaar [:jrmuizel] 2011-05-24 17:03:28 PDT
Fixed it. It was a problem of allocating using a different type than it actually was.
Comment 76 Frederic Bezies 2011-05-25 21:48:22 PDT
Sorry to spam this bug, but could bug #659676 be related to this one ?
Comment 77 :Ehsan Akhgari 2011-05-26 12:17:41 PDT
Backed out in http://hg.mozilla.org/mozilla-central/rev/a98c00e70590 because of possible Mac OS X 10.5 Dromaeo regression.

I'll reland tomorrow if this regression proves to not be the fault of this patch.
Comment 78 Jim Jeffery not reading bug-mail 1/2/11 2011-05-26 12:33:43 PDT
I see bugzilla is still seriouly F*ucked up...all I did was CC myself and the stupid bug closed...

reopening.
Comment 79 Daniel Holbert [:dholbert] 2011-05-26 14:53:26 PDT
(yeah, that bugzilla quirk is a known issue.  basically, if your bug tweak midair-collides with a change to any other fields, then your commit will set those fields back to their old values, unless you shift-reload.  Usually the "midair collision" splash page is a good heads-up that you should go back and shift-reload. It sucks, I know.)
Comment 80 Karl Tomlinson (:karlt) 2011-05-26 21:28:05 PDT
I don't know what shift-reload does, but normal reload is bad because remembers all the form settings and reapplies them after someone has changed any.
Comment 82 Jeff Muizelaar [:jrmuizel] 2011-06-01 11:58:28 PDT
Created attachment 536685 [details] [diff] [review]
Delete some code we don't need
Comment 83 Jeff Muizelaar [:jrmuizel] 2011-06-01 15:47:45 PDT
Created attachment 536764 [details] [diff] [review]
Inline the check method differently
Comment 84 Adrian Johnson 2011-06-22 06:46:02 PDT
The cairo version in cairo-features.h.in is still 1.9.5. The version in gfx/cairo/README has also not been updated.
Comment 85 JP Rosevear [:jpr] 2011-12-12 17:19:25 PST
*** Bug 554142 has been marked as a duplicate of this bug. ***
Comment 86 neil@parkwaycc.co.uk 2014-06-16 02:29:41 PDT
Comment on attachment 534865 [details] [diff] [review]
rebased again

>-			 (LPSTR) &lpMsgBuf,
>+			 (LPWSTR) &lpMsgBuf,
> 			 0, NULL)) {
> 	fprintf (stderr, "%s: Unknown GDI error", context);
>     } else {
>-	fprintf (stderr, "%s: %S", context, (char *)lpMsgBuf);
>+	fwprintf (stderr, L"%s: %S", context, (wchar_t *)lpMsgBuf);
So, this line originally started out as fprintf (stderr, "%s: %s", context, lpMsgBuf); where the message was retrieved using FormatMessageA and was therefore in ANSI.
Bug 458496 then switched to using FormatMessageW and fwprintf and used "%S: %s" as the fwprintf format. However this should have been a wide string and so bug 624198 switched it back to fprintf now using "%s: %S" as the format, which does at least work.
This patch then switched back to fwprintf but only widened the format string without correcting it to L"%S: %s".
Comment 87 neil@parkwaycc.co.uk 2014-06-16 02:42:22 PDT
Ah, it turns out that this was ignored in bug 710976.

Note You need to log in before you can comment on or make changes to this bug.