467423 - Painting stops in this case, using -moz-transform: scale, rotate and video

Status: VERIFIED FIXED

(Core :: Graphics, defect, P3)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when I visit that page, things in the page don't paint anymore. Visiting another url doesn't show up anything anymore.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I can reproduce this on Windows but not on Linux.

Maybe cairo is getting into an error state?

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: stack (with notes) showing cairo error

This stack shows the stack for the cairo error, with some notes I made based on looking at variables in the debugger.

I think this is probably something wrong with the NEARLY_ZERO handling in _compute_transform in cairo-win32-font.c, though somebody who knows that code better should have a look.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Reassigning, since this looks like a cairo bug.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Jeff, can you take a look at this?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, the NEARLY_ZERO stuff makes a bit more sense to me.  What's messing things up here is that in this case the non-zero components are so small they're NEARLY_ZERO, whereas the NEARLY_ZERO tests are assuming that the other components are not nearly zero (whereas in this case they are *actually* zero).

Comment 6

[Jeff Muizelaar [:jrmuizel]]

I don't think that this _cairo_error() (in cairo_matrix_invert() is the _cairo_error() that is causing rendering to stop. _compute_transform() does not propagate the error and has code for dealing with it. I think there must be an _cairo_error() later on that is causing rendering to stop.

Vlad, what should _compute_transform() be doing if it is passed a matrix that is scaled close to 0?

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Also, this bug seems to imply that we are reusing cairo_context_t on different pages. We should probably avoid doing so, so that the symptoms of a bug like this are not as catastrophic.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #7)
> Also, this bug seems to imply that we are reusing cairo_context_t on different
> pages. We should probably avoid doing so, so that the symptoms of a bug like
> this are not as catastrophic.

That's the thing though -- we create a new gfxContext/cairo_t on every expose event.  We even create a new surface on every expose event.  So I'm confused as to why all subsequent painting is broken. It could be something win32-specific as joe suggested, e.g. we're running out of GDI objects, but I just checked the GDI object count and it's fine.

Also note that not all subsequent painting is broken -- if you open a new tab and then load another page, it doesn't paint, but if you move the mouse over it parts of it are repainted.

I'd suggest stepping through the expose handler in the win32 nsWindow.cpp and seeing what's happening/where things are dying.  Maybe some font thing has an error on it?  That's the only resource that I can think of that would be kept around between expose events that could have an error impact.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

It looks like we fail out in _win32_scaled_font_set_world_transform when we call SetWorldTransform() with a bad transform.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #8)
> I'd suggest stepping through the expose handler in the win32 nsWindow.cpp and
> seeing what's happening/where things are dying.  Maybe some font thing has an
> error on it?  That's the only resource that I can think of that would be kept
> around between expose events that could have an error impact.

It looks like it's a scaled_font with a font_face with bad status that's causing things to stop working on other pages.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Do you know how it gets into that error state?  We should probably add some checking to the cache lookup code to look at font status, and to remove it from the cache (and recreate) if it has bad status, but that's probably something that should be split off from this bug.

Comment 12

[Jeff Muizelaar [:jrmuizel]]

The error state comes from having a font_face->backend->scaled_font_create() fail.
This causes the the font_face to be put into an error state. I'm not convinced that putting the font_face into an error state is really the correct thing to do...

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Attachment: Fix _compute_transform to check for nearly degenerate matrices [Checkin: Comment 16 & 18]

This patch fixes the rectangular matrix detection in _compute_transform. This avoids the problem triggered by the test case, but I wouldn't be surprised if the code is still fragile with degenerate matrices. I'll continue to look into improving that.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 353454 [details] [diff] [review]
Fix _compute_transform to check for nearly degenerate matrices
[Checkin: Comment 16 & 18]

Looks good!

Comment 15

[Jeff Muizelaar [:jrmuizel]]

I've also pushed the fix to cairo upstream.

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 353454 [details] [diff] [review]
Fix _compute_transform to check for nearly degenerate matrices
[Checkin: Comment 16 & 18]

http://hg.mozilla.org/mozilla-central/rev/8fb487d20112

Comment 17

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20081220 Minefield/3.2a1pre

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 353454 [details] [diff] [review]
Fix _compute_transform to check for nearly degenerate matrices
[Checkin: Comment 16 & 18]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/24e9186a2134