562746 - Update cairo to 1.10

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

This update should get us up-to-date with last cairo trunk

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: First try

Comment 2

[Jeff Muizelaar [:jrmuizel]]

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]]

Attachment: v2

Fixes some d2d compilation problems and updates to cairo trunk

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Attachment: v3

More updates. Doesn't build on win32 for some reason.

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: Additional patch to fix qt port compilation

Comment 6

[Robert Longson [:longsonr]]

(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]]

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)]

 > 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]]

Not really, it will cause a bunch of orange. Help fixing the issues would be great though :)

Comment 10

[Jeff Muizelaar [:jrmuizel]]

(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 12

[timeless]

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]

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]

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] (needinfo me, slow to respond)]

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]]

(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]]

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]]

Attachment: v4 - timeless' concerns not yet addressed

Comment 19

[Jeff Muizelaar [:jrmuizel]]

> >+
> >+	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]]

(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]]

(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]]

Karl,

Any guesses why we could get this kind of rendering: http://pastebin.mozilla.org/824349 ?

Comment 23

[Karl Tomlinson (:karlt)]

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]]

Looks like it was caused by:

commit 6b77567b6ef28710c7707ab82c7fa95c810152d1
Author: Chris Wilson <[e-mail]>
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]]

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]]

It seems like 3b1c0a4bd66660780095e6016e3db451f34503a3 is the other commit that broke a win32 test.

Comment 27

[Jeff Muizelaar [:jrmuizel]]

This could be a lot of work to fix...

Comment 28

[Jeff Muizelaar [:jrmuizel]]

Attachment: v5

Comment 29

[Joe Drew (not getting mail)]

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)]

(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)]

on www.ask.com Fennec show partially black  screen with cairo 1.10

Comment 32

[LegNeato]

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]]

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]

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]

Attachment: 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)]

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?)]

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]

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]

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]

(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)]

> 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?)]

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]

Yep, was just about to do that.

Comment 44

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Going to move the fennec deps here to bug 638594, apologies for bug spam.

Comment 45

[Brad Lassey [:blassey] (use needinfo?)]

removing the blocking flag from this and adding it to the bug that cjones just filed

Comment 46

[Oleg Romashin (:romaxa)]

Attachment: Cairo Qt fix

Comment 47

[Oleg Romashin (:romaxa)]

Comment on attachment 516749 [details] [diff] [review]
Cairo Qt fix


oh, /gfx/thebes/gfxQPainterSurface.cpp - part is wrong

Comment 48

[Oleg Romashin (:romaxa)]

Attachment: Cairo Qt fix v2

this compiles fine

Comment 49

[Timothy Nikkel (:tnikkel)]

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]

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]]

Attachment: rebased on top of 4e771e65764a

Comment 52

[Markus Stange [:mstange]]

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]]

Attachment: rebased on top of 4e771e65764a v2

This one is more likely to build.

Comment 54

[Jeff Muizelaar [:jrmuizel]]

I'm seeing a reftest failure on OS X on test bugs/615121-1.html

Comment 55

[Jeff Muizelaar [:jrmuizel]]

(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]]

Attachment: rebased on top of 4f07bebb993b

Comment 57

[Jeff Muizelaar [:jrmuizel]]

Attachment: rebased on top of 4f07bebb993b v2

This fixes the quartz issue and adds a buggy version of gdi changes.

Comment 58

[Jeff Muizelaar [:jrmuizel]]

an alternative approach to fallback:
http://people.mozilla.com/~jmuizelaar/cairo-update-march

Comment 59

[Jeff Muizelaar [:jrmuizel]]

http://people.mozilla.com/~jmuizelaar/cairo-update-march2

Comment 60

[Jeff Muizelaar [:jrmuizel]]

Seems like there's a memory leak when running reftests with d2d.

Comment 61

[Jeff Muizelaar [:jrmuizel]]

Attachment: rebased again

Comment 62

[(no longer active)]

Attachment: Rebased (for real this time)

Comment 63

[(no longer active)]

Attachment: 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]]

Attachment: 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]]

Attachment: 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]]

Attachment: Add reftest that fails with the quartz backend from cairo 1.10

Comment 67

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add reftest that fails with the quartz backend from cairo 1.10

Comment 68

[(no longer active)]

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) (email my personal email if necessary)]

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]]

Attachment: rebased on top 1f3777d4ed8b

Comment 71

[Jeff Muizelaar [:jrmuizel]]

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]]

Attachment: 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]]

Attachment: 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]]

Fixed it. It was a problem of allocating using a different type than it actually was.

Comment 75

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/mozilla-central/rev/989fe2906b77
http://hg.mozilla.org/mozilla-central/rev/db4b1f3096ea
http://hg.mozilla.org/mozilla-central/rev/acb4e51fa8a6

Comment 76

[Frederic Bezies]

Sorry to spam this bug, but could bug #659676 be related to this one ?

Comment 77

[(no longer active)]

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]

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]]

(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)]

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 81

[(no longer active)]

Relanded:

http://hg.mozilla.org/mozilla-central/rev/877f11b44bfe
http://hg.mozilla.org/mozilla-central/rev/05b2836292b2
http://hg.mozilla.org/mozilla-central/rev/102be3d1f103

Comment 82

[Jeff Muizelaar [:jrmuizel]]

Attachment: Delete some code we don't need

Comment 83

[Jeff Muizelaar [:jrmuizel]]

Attachment: Inline the check method differently

Comment 84

[u285178]

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 86

[[e-mail]]

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

[[e-mail]]

Ah, it turns out that this was ignored in bug 710976.