The default bug view has changed. See this FAQ.

Update cairo to 1.10

RESOLVED FIXED in mozilla7

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

(Depends on: 13 bugs, Blocks: 2 bugs)

Trunk
mozilla7
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(7 attachments, 18 obsolete attachments)

1.95 MB, patch
Details | Diff | Splinter Review
966 bytes, patch
Details | Diff | Splinter Review
394 bytes, text/html
Details
1.06 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.91 KB, patch
Ehsan
: review+
Details | Diff | Splinter Review
2.04 MB, patch
Details | Diff | Splinter Review
1.43 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
This update should get us up-to-date with last cairo trunk
(Assignee)

Updated

7 years ago
Blocks: 545632
(Assignee)

Comment 1

7 years ago
Created attachment 443664 [details] [diff] [review]
First try
(Assignee)

Updated

7 years ago
Blocks: 526977
(Assignee)

Comment 2

7 years ago
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 |
(Assignee)

Comment 3

7 years ago
Created attachment 444016 [details] [diff] [review]
v2

Fixes some d2d compilation problems and updates to cairo trunk
Attachment #443664 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 563255
(Assignee)

Comment 4

7 years ago
Created attachment 447523 [details] [diff] [review]
v3

More updates. Doesn't build on win32 for some reason.
Attachment #444016 - Attachment is obsolete: true
Blocks: 569669
Created attachment 448992 [details] [diff] [review]
Additional patch to fix qt port compilation

Comment 6

7 years ago
(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
(Assignee)

Comment 7

7 years ago
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
 > layout/reftests/bugs/179596-2.html

Can we just push new cairo update, and record all known issues as bugs ?
(Assignee)

Comment 9

7 years ago
Not really, it will cause a bunch of orange. Help fixing the issues would be great though :)
Blocks: 566283
(Assignee)

Comment 10

7 years ago
(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

Updated

7 years ago
Duplicate of this bug: 526977

Updated

7 years ago
Blocks: 583135

Comment 12

7 years ago
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.
Attachment #447523 - Flags: feedback-

Comment 13

7 years ago
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);
Attachment #447523 - Flags: feedback-

Comment 14

7 years ago
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);
Attachment #447523 - Flags: feedback-
(Assignee)

Updated

7 years ago
Summary: Update cairo to upstream → Update cairo to 1.10
Can you also, as part of this, normalize line endings in the d2d code?  That is, remove all the dos line endings...
(Assignee)

Comment 16

7 years ago
(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.
(Assignee)

Comment 17

7 years ago
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
(Assignee)

Comment 18

7 years ago
Created attachment 485329 [details] [diff] [review]
v4 - timeless' concerns not yet addressed
Attachment #447523 - Attachment is obsolete: true
(Assignee)

Comment 19

7 years ago
> >+
> >+	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
(Assignee)

Comment 20

7 years ago
(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
(Assignee)

Comment 21

7 years ago
(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.
(Assignee)

Comment 22

7 years ago
Karl,

Any guesses why we could get this kind of rendering: http://pastebin.mozilla.org/824349 ?
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?
(Assignee)

Comment 24

7 years ago
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.
(Assignee)

Comment 25

7 years ago
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);
(Assignee)

Comment 26

7 years ago
It seems like 3b1c0a4bd66660780095e6016e3db451f34503a3 is the other commit that broke a win32 test.
(Assignee)

Comment 27

6 years ago
This could be a lot of work to fix...
(Assignee)

Comment 28

6 years ago
Created attachment 490975 [details] [diff] [review]
v5
Attachment #485329 - Attachment is obsolete: true

Updated

6 years ago
Blocks: 601512
blocking2.0: --- → ?
Right now I don't think we absolutely require Cairo 1.10. If it becomes a hard blocker for anyone, this could change.
blocking2.0: ? → -
Depends on: 616700
(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 ;-)
on www.ask.com Fennec show partially black  screen with cairo 1.10

Comment 32

6 years ago
Bug 601512 is a hardblocker that says it will be fixed by this bug. I guess this needs to hardblock as well, correct?
Blocks: 629012
Would this also repair bug 595671, based on comments in that bug?
Blocks: 637828
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.
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.
Blocks: 610344
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.
Blocks: 616638
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
No longer blocks: 616638
tracking-fennec: --- → ?
Blocks: 616638
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.
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.
(For future reference, this patch doesn't build on maemo-qt: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1299185767.1299187744.7888.gz .)
> reply to comment #40)
I guess you need https://bugzilla.mozilla.org/attachment.cgi?id=448992&action=edit(In
cjones, can you file a bug for taking that subset of patches? and leave this one for the full update.
tracking-fennec: ? → 2.0+
Yep, was just about to do that.
Going to move the fennec deps here to bug 638594, apologies for bug spam.
No longer blocks: 569669, 583135, 610344, 616638, 637828
removing the blocking flag from this and adding it to the bug that cjones just filed
tracking-fennec: 2.0+ → ---
Depends on: 638594
Created attachment 516749 [details] [diff] [review]
Cairo Qt fix
Attachment #448992 - Attachment is obsolete: true
Comment on attachment 516749 [details] [diff] [review]
Cairo Qt fix


oh, /gfx/thebes/gfxQPainterSurface.cpp - part is wrong
Created attachment 516750 [details] [diff] [review]
Cairo Qt fix v2

this compiles fine
Attachment #516749 - Attachment is obsolete: true
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...
That's OK, I'm not 100% confident in my rebasing either :).  This was more for proof-of-concept purposes.
(Assignee)

Comment 51

6 years ago
Created attachment 519213 [details] [diff] [review]
rebased on top of 4e771e65764a
Attachment #490975 - Attachment is obsolete: true
Comment on attachment 519213 [details] [diff] [review]
rebased on top of 4e771e65764a

The added files are missing in this patch.
(Assignee)

Comment 53

6 years ago
Created attachment 519411 [details] [diff] [review]
rebased on top of 4e771e65764a v2

This one is more likely to build.
Attachment #519213 - Attachment is obsolete: true
(Assignee)

Comment 54

6 years ago
I'm seeing a reftest failure on OS X on test bugs/615121-1.html
(Assignee)

Comment 55

6 years ago
(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.
(Assignee)

Comment 56

6 years ago
Created attachment 519737 [details] [diff] [review]
rebased on top of 4f07bebb993b
Attachment #519411 - Attachment is obsolete: true
(Assignee)

Comment 57

6 years ago
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.
Attachment #519737 - Attachment is obsolete: true
(Assignee)

Comment 58

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

Updated

6 years ago
Blocks: 614721
(Assignee)

Comment 59

6 years ago
http://people.mozilla.com/~jmuizelaar/cairo-update-march2
(Assignee)

Comment 60

6 years ago
Seems like there's a memory leak when running reftests with d2d.
(Assignee)

Comment 61

6 years ago
Created attachment 531318 [details] [diff] [review]
rebased again
Attachment #521189 - Attachment is obsolete: true
Created attachment 531400 [details] [diff] [review]
Rebased (for real this time)
Attachment #531318 - Attachment is obsolete: true
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.
(Assignee)

Updated

6 years ago
Depends on: 656465
(Assignee)

Comment 64

6 years ago
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
Attachment #531400 - Attachment is obsolete: true
(Assignee)

Comment 65

6 years ago
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.
Attachment #532207 - Flags: review?(roc)
(Assignee)

Comment 66

6 years ago
Created attachment 532241 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10
Attachment #532241 - Flags: review?(ehsan)
(Assignee)

Comment 67

6 years ago
Created attachment 532243 [details] [diff] [review]
Add reftest that fails with the quartz backend from cairo 1.10
Attachment #532241 - Attachment is obsolete: true
Attachment #532243 - Flags: review?(ehsan)
Attachment #532241 - Flags: review?(ehsan)
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.
Attachment #532243 - Flags: review?(ehsan) → review+
Comment on attachment 532207 [details] [diff] [review]
Remove use of EXTEND_NONE on quartz

Review of attachment 532207 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #532207 - Flags: review?(roc) → review+
(Assignee)

Comment 70

6 years ago
Created attachment 533382 [details] [diff] [review]
rebased on top 1f3777d4ed8b
Attachment #531780 - Attachment is obsolete: true
(Assignee)

Comment 71

6 years ago
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.
(Assignee)

Comment 72

6 years ago
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
Attachment #533382 - Attachment is obsolete: true
(Assignee)

Comment 73

6 years ago
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
Attachment #533949 - Attachment is obsolete: true
(Assignee)

Comment 74

6 years ago
Fixed it. It was a problem of allocating using a different type than it actually was.
(Assignee)

Comment 75

6 years ago
http://hg.mozilla.org/mozilla-central/rev/989fe2906b77
http://hg.mozilla.org/mozilla-central/rev/db4b1f3096ea
http://hg.mozilla.org/mozilla-central/rev/acb4e51fa8a6
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 76

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

Updated

6 years ago
Depends on: 659676
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
I see bugzilla is still seriouly F*ucked up...all I did was CC myself and the stupid bug closed...

reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(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.)
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.
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
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Depends on: 660448
Blocks: 646611
Depends on: 456448
(Assignee)

Comment 82

6 years ago
Created attachment 536685 [details] [diff] [review]
Delete some code we don't need
(Assignee)

Comment 83

6 years ago
Created attachment 536764 [details] [diff] [review]
Inline the check method differently
Attachment #536685 - Attachment is obsolete: true
Depends on: 663258

Updated

6 years ago
Assignee: nobody → jmuizelaar

Comment 84

6 years ago
The cairo version in cairo-features.h.in is still 1.9.5. The version in gfx/cairo/README has also not been updated.

Updated

6 years ago
Depends on: 668549

Updated

6 years ago
Depends on: 671302
Depends on: 671064

Updated

6 years ago
Depends on: 689217
Depends on: 689962

Updated

6 years ago
Depends on: 691167
Depends on: 692350
Blocks: 404637
Depends on: 698007

Updated

6 years ago
Depends on: 700003

Updated

5 years ago
Duplicate of this bug: 554142

Updated

5 years ago
Depends on: 716462
Depends on: 720035

Updated

5 years ago
Depends on: 725937

Updated

5 years ago
Depends on: 730571

Updated

5 years ago
Depends on: 733714
Blocks: 567370

Updated

5 years ago
Depends on: 801794

Updated

5 years ago
Depends on: 808249

Updated

4 years ago
Depends on: 768362

Updated

4 years ago
Depends on: 873313
Depends on: 877492

Updated

4 years ago
Depends on: 896537

Updated

4 years ago
Depends on: 886642

Updated

3 years ago
No longer depends on: 886642
Depends on: 974310

Updated

3 years ago
Depends on: 989592
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".
Ah, it turns out that this was ignored in bug 710976.

Updated

3 years ago
Depends on: 672726
You need to log in before you can comment on or make changes to this bug.