Bas, Can you review the direct2d/directwrite changes?

One known problem with this is a hang/crash on linux/tsvg

Comment on attachment 427655 [details] [diff] [review] v2 fixes a bunch of test failures >@@ -244,20 +244,17 @@ _cairo_d2d_show_glyphs (void *surface, > * \param extents Pointer to where to store the extents > * \param CAIRO_ERROR_SURFACE_TYPE_MISTMATCH or CAIRO_STATUS_SUCCESS > */ >-static cairo_int_status_t >+static cairo_bool_t > _cairo_d2d_getextents(void *surface, > cairo_rectangle_int_t *extents); > Nit: The backend renamed this to get_extents, we should rename the function too for consistency. >@@ -1605,9 +1606,11 @@ static cairo_int_status_t > _cairo_d2d_paint(void *surface, > cairo_operator_t op, > const cairo_pattern_t *source, >- cairo_rectangle_int_t *extents) >+ cairo_clip_t *clip) > { > cairo_d2d_surface_t *d2dsurf = static_cast<cairo_d2d_surface_t*>(surface); >+ cairo_int_status_t status; >+ > _begin_draw_state(d2dsurf); > if (op == CAIRO_OPERATOR_CLEAR) { > cairo_solid_pattern_t *sourcePattern = >@@ -1617,6 +1620,10 @@ _cairo_d2d_paint(void *surface, > return CAIRO_INT_STATUS_SUCCESS; > } > >+ status = (cairo_int_status_t)_cairo_surface_clipper_set_clip (&d2dsurf->clipper, clip); >+ if (unlikely(status)) >+ return status; >+ This needs to happen before _begin_draw_state in all functions. Depending on the function it might be best to move _begin_draw_state down or move this one up. _begin_draw_state actually pushes the clip. In this order the clip will never be applied. Note the glyph drawing code deserves particular attention. Since clipper_set_clip is done in cairo-dwrite-font.cpp but _begin_draw_state is currently done in cairo-d2d-surface.cpp

We should enable the Cairo tests in our tree once this update is finished. (And maybe the cairo trace suites in talos?)

Bas, can you review the d2d changes?

It feels like something is still wrong with x stuff... (firefox-bin:6685): Gdk-CRITICAL **: gdk_screen_get_display: assertion `GDK_IS_SCREEN (screen)' failed (firefox-bin:6685): GLib-GObject-WARNING **: invalid (NULL) pointer instance (firefox-bin:6685): GLib-GObject-CRITICAL **: g_signal_handlers_disconnect_matched: assertion `G_TYPE_CHECK_INSTANCE (instance)' failed (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed (firefox-bin:6685): Gdk-CRITICAL **: gdk_screen_get_display: assertion `GDK_IS_SCREEN (screen)' failed (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_get_data: assertion `G_IS_OBJECT (object)' failed (firefox-bin:6685): Gdk-CRITICAL **: gdk_screen_get_display: assertion `GDK_IS_SCREEN (screen)' failed (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_get_data: assertion `G_IS_OBJECT (object)' failed (firefox-bin:6685): Gdk-CRITICAL **: gdk_screen_get_display: assertion `GDK_IS_SCREEN (screen)' failed (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_get_data: assertion `G_IS_OBJECT (object)' failed (firefox-bin:6685): GLib-GObject-WARNING **: /build/buildd/glib2.0-2.14.1/gobject/gsignal.c:2180: invalid unclassed object pointer for value type `GdkScreen' (firefox-bin:6685): Gdk-CRITICAL **: gdk_screen_get_display: assertion `GDK_IS_SCREEN (screen)' failed (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_get_data: assertion `G_IS_OBJECT (object)' failed (firefox-bin:6685): GLib-GObject-WARNING **: /build/buildd/glib2.0-2.14.1/gobject/gsignal.c:2180: invalid unclassed object pointer for value type `GdkScreen' (firefox-bin:6685): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

This causes a tsvg regression on linux. The largest change happens in hixie-007.xml which is a stroked world map. Here are the results: old: hixie-007.xml;1613;1612.25;1609;1629;1609;1629;1612;1614;1614 new: hixie-007.xml;3973;3972.75;3972;4006;3973;4006;3973;3972;3973 I believe this is caused by running the tesellator on stroked paths, which is a correctness fix. If we disable stroking we get the following results: old: hixie-007.xml;1100;1099 ;1093;1111;1111;1103;1093;1097;1103 new: hixie-007.xml;1039;1038.25;1034;1050;1034;1040;1041;1050;1038 which seems to confirm the hypothesis. None of the other platforms should suffer from this problem.

If it's linux only, that's unfortunate, but probably fine...

We also seem to have a tp4 regression on linux. It seems to be worst on chinese pages we makes me lean towards the glyph cache, however I don't know that there have been many changes in that area. This might be tricky to track down...

(In reply to comment #11) > We also seem to have a tp4 regression on linux. It seems to be worst on chinese > pages we makes me lean towards the glyph cache, however I don't know that there > have been many changes in that area. This might be tricky to track down... This patch includes large chunks of changes which have some things to do with the glyph cache. I think at its current state, it is extremely hard to tell what's wrong with this patch. I also know that bisecting against cairo changesets is tricky because we have to merge our cairo patches. Jeff, what does shark on the chinese pages tell you?

(In reply to comment #12) > (In reply to comment #11) > > We also seem to have a tp4 regression on linux. It seems to be worst on chinese > > pages we makes me lean towards the glyph cache, however I don't know that there > > have been many changes in that area. This might be tricky to track down... > > This patch includes large chunks of changes which have some things to do with > the glyph cache. I think at its current state, it is extremely hard to tell > what's wrong with this patch. I also know that bisecting against cairo > changesets is tricky because we have to merge our cairo patches. > > Jeff, what does shark on the chinese pages tell you? The problem is only noticeable on linux (the only place we use the glyph cache). I'll do some glyph cache instrumentation tomorrow to see if I can find a change in behaviour.

This causes a significant (10-20%) performance regression for Direct2D. Do we have any idea what might be causing this? Do we somehow perhaps call the clip function more often or something of the likes?

(In reply to comment #14) > This causes a significant (10-20%) performance regression for Direct2D. Do we > have any idea what might be causing this? What gets worse? > Do we somehow perhaps call the clip > function more often or something of the likes? Perhaps. If I can reproduce the regression, I'll have a look.

(In reply to comment #15) > (In reply to comment #14) > > This causes a significant (10-20%) performance regression for Direct2D. Do we > > have any idea what might be causing this? > > What gets worse? > > > Do we somehow perhaps call the clip > > function more often or something of the likes? > > Perhaps. If I can reproduce the regression, I'll have a look. Pretty much everything gets somewhat slower. It's especially noticable in debug mode when the D2D debug layer is turned on.

I have been digging into the linux tp4 regressions. I've disabled progressively more of the drawing and yet the regression persists: baseline: http://people.mozilla.org/~jmuizelaar/graph-server/detail.html?host=graphs-stage&old=1258771&new=1258807 disabled text, stroking and filling: http://people.mozilla.org/~jmuizelaar/graph-server/detail.html?host=graphs-stage&old=1259691&new=1259879 disabled text, stroking, filling and painting (pretty much everything) http://people.mozilla.org/~jmuizelaar/graph-server/detail.html?host=graphs-stage&old=1260459&new=1260531 I'm not exactly sure how to proceed.

I can not reproduce the d2d slowdown. Here are the times I get with the following bookmarklet: javascript:var%20r%20=%20window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIDOMWindowUtils).redraw(500);%20alert(r%20+%20'%20ms'); old gmail 5657ms cnn 4023 ms mozilla 5741 ms tiger - 40 fps css-radius.heroku.com 3753 ms, 3745 ms, 3740 ms new cnn 3870 ms mozilla 4832 ms gmail 5119 ms tiger - 42 fps css-radius.heroku.com 3732 ms, 3746 ms

Comment on attachment 432638 [details] [diff] [review] getting closer A review of adding Extend(PAD_EDGE) to the svg code wouldn't hurt.

We need to use EXTEND_PAD on linux to get the reftests to pass with the cairo update. Using EXTEND_PAD_EDGE will do the right thing on all the platforms.

Comment on attachment 432638 [details] [diff] [review] getting closer yay

We need to try harder to trick cairo into applying the clip to the destination surface.

Backed out for a windows build failure and some weird reftest failures on mac REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/382916-1.html | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/383488-1.html | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/383883-1.html | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/383883-3.html | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/383884-1.html | REFTEST TEST-UNEXPECTED-FAIL | file:///builds/moz2_slave/mozilla-central-macosx-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/384322-1.html |

It seems as though the contents from the left frame of layout/reftests/bugs/381746-1.html are persisting into the following tests. Anyone have any idea why this would happen?

Try this

So more info: - It looks like the problem comes from rebasing mozilla-central. Using m-c from March 12 passes the test. - 381746-1.html, the frameset test, changed in this range - one of the failing tests was also added. - It's not clear whether the failure is new or if we're just exposing a problem that was also in the March 12 m-c.

Reverting the changes to 381746-1.html seems to make the problem go away.

gfxASurface::gfxSurfaceType should be updated in the patch to reflect the new Cairo surface types. It's already missing SurfaceTypeMeta and doesn't have D2D at the end (although the former is worse since it breaks DDraw). We should aim to keep it up to date probably.

Here's a paint log of the failing reftest: http://pastebin.mozilla.org/713573

So my latest theory about the problem is that the clip when we draw the second frame in 381746-1.html is not be restored after drawWindow completes. This causes the followings tests to be clipped to the area of the second frame until the clip is restored.

The landing on m-c is showing painting / image issues. Visit: http://forums.mozillazine.org/viewforum.php?f=23 Note the icons on the left side of the forum are not displayed, but dragging the mouse down the page causes them to appear, and dragging it up the page they go away. I verified the regression range to the landing of this update: 20100407201605 0770fe34e535 ok 20100407204139 9480726de986 bad cairo update This is going to make the nightly builds pretty bad. :(

I'm also seeing a lot of painting problems (black squares in many places where new painting should happen, which don't get fully replaces by the actual content) in a current SeaMonkey trunk build on Linux. This checkin seems to be the most likely to cause that...

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100408 Minefield/3.7a5pre There doesn't appear to be a comment here with a changeset reference for the checkin. In fact, there are no changeset references for any of the checkins for this bug. On Linux there are black rectangles all over the place, in content and in chrome. The scroll buttons, selected text, and also randomly. Caret doesn't seem to unpaint in textareas. Scrolling a black rectangle out of view and back in again seems to repaint and remove it. Please respin!

I'm not seeing these problems. d2d/dw enabled for me. Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100408 Minefield/3.7a5pre - Build ID: 20100408041901

For grins, I just enabled directwrite and Direct2D and the paint problems seems to have gone away. Interesting!

Has anyone seen this problem on OS X?

I've backed this out.

Perhaps not so important info, but in Gmail, the check boxes by each message do not appear except on mouse pointer hover. Then, some remain while others disappear after the pointer moves off a check box.

When you land it again, could you make sure to list the new base version of cairo in gfx/cairo/README (it's not 1.8.2 any more)?

(In reply to comment #41) > Created an attachment (id=439369) [details] > A reftest for a problem that our current tests didn't catch It looks like the reftest won't show the problem because of when we do drawWindow we always have a clip.

(In reply to comment #42) > > It looks like the reftest won't show the problem because of when we do > drawWindow we always have a clip. Unlike when we do drawing to an actual window.

I'm getting some new unexplained reftest failures on win32. REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/slave/win32-unit/mozilla/layout/reftests/font-face/synthetic-weight-style.html | REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/slave/win32-unit/mozilla/layout/reftests/font-face/synthetic-variations.html | REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/slave/win32-unit/mozilla/layout/reftests/margin-collapsing/block-clear-6e-left.html | REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/slave/win32-unit/mozilla/layout/reftests/margin-collapsing/block-clear-6f-left.html |

Let's try again: http://hg.mozilla.org/mozilla-central/rev/f236632a9747

Still not working right. Using the latest hourly build with the update, images on some forums, CNN, MSNBC images are not appearing, but do when the page is scrolled. Addon Icons on the NavBar go transparent when hovered over. Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100426 Minefield/3.7a5pre ID:20100426084628

FavIcons on bookmarks are not showing either.

A quick test reveals that things seem to be ok with DW/D2D enabled, but those 'off', the issue I stated above shows up.

(In reply to comment #48) > A quick test reveals that things seem to be ok with DW/D2D enabled, but those > 'off', the issue I stated above shows up. Can you post a screenshot of this issue?

(In reply to comment #49) > (In reply to comment #48) > > A quick test reveals that things seem to be ok with DW/D2D enabled, but those > > 'off', the issue I stated above shows up. > > Can you post a screenshot of this issue? Will be tomorrow, I just now arrived at work..

When I fixed the win32-surface show_glyphs function I didn't fix dwrite parallel. This does so.

Here's a site having painting issues when scrolling up and down: http://www.history.ca/video/default.aspx This is with all dw/d2d stuff OFF

(In reply to comment #52) > Here's a site having painting issues when scrolling up and down: > http://www.history.ca/video/default.aspx > > This is with all dw/d2d stuff OFF I'm hoping this is fixed by the fix to bug 561827. If not reopen.

Don't reopen! File more bugs and make them dependent. Congrats on landing Cairo, Jeff!

I don't see the #include <intrin.h> anywhere in these patches although it appears in cairo.c?

(In reply to comment #55) > I don't see the #include <intrin.h> anywhere in these patches although it > appears in cairo.c? Indeed. I neglected to update that patch. I'll fix it when I merge upstream.