Mike Hommey [:glandium] Reporter
	Description • 10 years ago

https://tbpl.mozilla.org/php/getParsedLog.php?id=48835200&tree=Elm

Test (CairoClip918671): VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
VERIFY FAILED: memcmp(dataSurf1->GetData() + y * dataSurf1->Stride(), dataSurf2->GetData() + y * dataSurf2->Stride(), dataSurf1->GetSize().width * 4) == 0
FAILED

GTK+3 builds use system cairo (no choice). This happens with cairo 1.10.2, but not with cairo 1.12.0. At least ubuntu 12.04 (LTS) comes with cairo 1.10.2.

In fact, this also happens on GTK+2 builds built with --enable-system-cairo and running against cairo 1.10.2, which suggests one of the patches we have in our in-tree cairo fixes the issue, and that the issue is also fixed in 1.12.0.

It is more than possible than many of the oranges on elm (especially reftest) are actually the same bug. https://tbpl.mozilla.org/?tree=Elm

	Mike Hommey [:glandium] Reporter
	Comment 1 • 10 years ago

And the patch in question is gfx/cairo/cairo-mask-extends-bug.patch, coming from bug 918671 (as the test name hints)

	Mike Hommey [:glandium] Reporter
	Comment 2 • 10 years ago

Now, the question is what to do with this?

	Mike Hommey [:glandium] Reporter
	Comment 3 • 10 years ago

Note that, thinking about it, even if the many oranges on elm are not this particular patch, many may well be the result of running with a system cairo 1.10.2 (test machines are running ubuntu 12.04) without our patches (obviously).

	Benoit Girard (:BenWa)
	Comment 4 • 10 years ago

I'm not familiar with Elm (links?) or why we're wanting to use system cairo and why we don't have a choice there.

The cairo bug has been fixed upstream so ideally we wouldn't use a system cairo if it's too old. Jeff has been following the cairo development so he would know better the implication of using various version of cairo as the system and the impact of using a cairo version that we aren't specifically targeting (including security consideration).

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 5 • 10 years ago

We're trying to move to system cairo as a precursor to going to Skia.  Our approach was going to be running with system cairo and seeing what fails - here's the one that fails.  I'm good with Jeff making a call if this is a real bug that upstream cairo needs fixed, something we can live with and disable the test, change our code to work around this problem, or do something else.

BenWa: https://tbpl.mozilla.org/?tree=Elm

	Jeff Muizelaar [:jrmuizel]
	Comment 6 • 10 years ago

Do we need to use system cairo when building with gtk3? Is there anything we can do to fix that?

	Mike Hommey [:glandium] Reporter
	Comment 7 • 10 years ago

IIRC, using the in-tree cairo with gtk3 itself using system cairo messes things up. Forcing gtk3 to use the in-tree cairo would require building it as a shared library, as well as pixman, and would likely break system libraries, possibly including gtk3, if the system cairo or system pixman are more recent than the one we bundle. We /could/ load our own cairo and pixman only if we detect that the system versions are older or equal (assuming that can be detected). That seems risky, though.

That said, if we get to move content to skia, that should get us automatically out of any of those problems, right? Is it possible to test that (even if it's not ready for prime-time)?

	Jeff Muizelaar [:jrmuizel]
	Comment 8 • 10 years ago

(In reply to Mike Hommey [:glandium] from comment #7)
> IIRC, using the in-tree cairo with gtk3 itself using system cairo messes
> things up. Forcing gtk3 to use the in-tree cairo would require building it
> as a shared library, as well as pixman, and would likely break system
> libraries, possibly including gtk3, if the system cairo or system pixman are
> more recent than the one we bundle. We /could/ load our own cairo and pixman
> only if we detect that the system versions are older or equal (assuming that
> can be detected). That seems risky, though.
> 
> That said, if we get to move content to skia, that should get us
> automatically out of any of those problems, right? Is it possible to test
> that (even if it's not ready for prime-time)?

It should be possible to have gtk3 use system cairo and us use in-tree cairo. As long as we don't try to interop between them we should be fine.

	Mike Hommey [:glandium] Reporter
	Comment 9 • 10 years ago

(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> (In reply to Mike Hommey [:glandium] from comment #7)
> > IIRC, using the in-tree cairo with gtk3 itself using system cairo messes
> > things up. Forcing gtk3 to use the in-tree cairo would require building it
> > as a shared library, as well as pixman, and would likely break system
> > libraries, possibly including gtk3, if the system cairo or system pixman are
> > more recent than the one we bundle. We /could/ load our own cairo and pixman
> > only if we detect that the system versions are older or equal (assuming that
> > can be detected). That seems risky, though.
> > 
> > That said, if we get to move content to skia, that should get us
> > automatically out of any of those problems, right? Is it possible to test
> > that (even if it's not ready for prime-time)?
> 
> It should be possible to have gtk3 use system cairo and us use in-tree
> cairo. As long as we don't try to interop between them we should be fine.

Martin, I think it was you who mentioned that didn't work out. Do you remember?

	Karl Tomlinson (:karlt)
	Comment 10 • 10 years ago

(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> It should be possible to have gtk3 use system cairo and us use in-tree
> cairo. As long as we don't try to interop between them we should be fine.

It would be possible to write code to make that work.  The interop that we try exists in native theme drawing and also iirc responding to native expose events.

We'd need something like gfxCairoNativeRenderer to allow the themes to draw, and something resembling an inverse to interpret the expose event clip, etc.

If the goal is to use skia for content rendering, then cairo use would be restricted to layers compositing, and glyph metrics, (and printing?) significantly reducing exposure to cairo bugs.

This particular bug was fixed in cairo-image-surface.c, so I'm expecting that it won't happen when using skia for content rendering.

I'd be inclined to annotate the known failures for now and reassess after the switch to skia.

There will always be bugs in the graphics stack, often particular to certain systems.
Having a different cairo in Gecko can protect us against some of those, but sometimes the system cairo can have workarounds to save us from bugs lower in the graphics stack.

	Martin Stránský [:stransky] (ni? me)
	Comment 11 • 10 years ago

It would help if we allow to build with in-tree cairo. Distros usually don't update system libraries regularly, especially in older releases.

	Mike Hommey [:glandium] Reporter
	Comment 12 • 9 years ago

Bug 1159273 made us use in-tree cairo.