Closed Bug 1222171 Opened 9 years ago Closed 9 years ago

"GraphicsCriticalError: |[0][GFX1]: Unknown cairo format 3" on startup on Linux machine with VNC "screen"


(Core :: Graphics, defect)

Not set



Tracking Status
firefox44 + verified
firefox45 --- verified
relnote-firefox --- 44+


(Reporter: mstange, Assigned: n.nethercote)



(Keywords: regression)


(1 file)

I'm seeing this startup crash when running a build on Jeff's "hawkman" machine in the Toronto office (which I use for rr debugging). The DISPLAY env variable is set to something that I get by running "vnc4server".

> #0  0x00007fffe45d3ff6 in mozilla::gfx::CriticalLogger::OutputMessage(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, bool) (aString="[GFX1]: Unknown cairo format 3", aLevel=1, aNoNewline=false) at /home/mstange/code/mozilla-central/gfx/2d/Factory.cpp:900
> #1  0x00007fffe4612329 in mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::WriteLog(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (this=0x7fffd2e966b0, aString="[GFX1]: Unknown cairo format 3") at /home/mstange/code/mozilla-central/gfx/2d/Logging.h:502
> #2  0x00007fffe460a902 in mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::Flush() (this=0x7fffd2e966b0) at /home/mstange/code/mozilla-central/gfx/2d/Logging.h:276
> #3  0x00007fffe45ff9fe in mozilla::gfx::Log<1, mozilla::gfx::CriticalLogger>::~Log() (this=0x7fffd2e966b0, __in_chrg=<optimized out>)
>     at /home/mstange/code/mozilla-central/gfx/2d/Logging.h:268
> #4  0x00007fffe45bfef8 in mozilla::gfx::CairoFormatToGfxFormat(cairo_format_t) (format=CAIRO_FORMAT_A1) at /home/mstange/code/mozilla-central/gfx/2d/HelpersCairo.h:246
> #5  0x00007fffe45c1c98 in mozilla::gfx::GfxFormatForCairoSurface(_cairo_surface*) (surface=0x7fffc28db200) at /home/mstange/code/mozilla-central/gfx/2d/DrawTargetCairo.cpp:668
> #6  0x00007fffe4863122 in gfxASurface::GetSurfaceFormat() const (this=0x7fffc28ff920) at /home/mstange/code/mozilla-central/gfx/thebes/gfxASurface.cpp:715
> #7  0x00007fffe4883d8c in gfxPlatform::CreateDrawTargetForSurface(gfxASurface*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) (this=0x7fffd49cea40, aSurface=0x7fffc28ff920, aSize=...) at /home/mstange/code/mozilla-central/gfx/thebes/gfxPlatform.cpp:786
> #8  0x00007fffe46f4aec in mozilla::layers::X11DataTextureSourceBasic::Update(mozilla::gfx::DataSourceSurface*, nsIntRegion*, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>*) (this=0x7fffc2b07ee0, aSurface=0x7fffc28ff8d0, aDestRegion=0x0, aSrcOffset=0x0) at /home/mstange/code/mozilla-central/gfx/layers/basic/X11BasicCompositor.cpp:47
> #9  0x00007fffe47c2f44 in mozilla::layers::BufferTextureHost::Upload(nsIntRegion*) (this=0x7fffc28d2d80, aRegion=0x0)
>     at /home/mstange/code/mozilla-central/gfx/layers/composite/TextureHost.cpp:621
> #10 0x00007fffe47c232d in mozilla::layers::BufferTextureHost::MaybeUpload(nsIntRegion*) (this=0x7fffc28d2d80, aRegion=0x0)
>     at /home/mstange/code/mozilla-central/gfx/layers/composite/TextureHost.cpp:504
> #11 0x00007fffe47c20db in mozilla::layers::BufferTextureHost::Lock() (this=0x7fffc28d2d80) at /home/mstange/code/mozilla-central/gfx/layers/composite/TextureHost.cpp:459
> #12 0x00007fffe47cdd0e in mozilla::layers::ContentHostTexture::Lock() (this=0x7fffc28b5df0) at /home/mstange/code/mozilla-central/gfx/layers/composite/ContentHost.h:145
> #13 0x00007fffe4707b77 in mozilla::layers::AutoLockCompositableHost::AutoLockCompositableHost(mozilla::layers::CompositableHost*) (this=0x7fffd2e96e00, aHost=0x7fffc28b5df0)
>     at ../../dist/include/CompositableHost.h:252

CairoFormatToGfxFormat complains that it doesn't know the format CAIRO_FORMAT_A1.
The image surface that we're looking at was created with CAIRO_FORMAT_A1 in gfxImageSurface::AllocateAndInit, where mFormat is gfxImageFormat::RGB16_565.
So gfxImageFormatToCairoFormat, which just reinterprets the integer value, wrongly casts gfxImageFormat::RGB16 to CAIRO_FORMAT_A1. This seems bad.

The gfxImageSurface is created with gfxImageFormat::RGB16_565 because that's what gfxPlatform::GetPlatform()->Optimal2DFormatForContent(gfxContentType::COLOR) returns when called from ContentClientRemoteBuffer::CreateBuffer - this value is coming from gfxPlatformGtk::GetOffscreenFormat() in the case that gdk_visual_get_depth(gdk_visual_get_system()) == 16. So it looks like this VNC setup simulates a 16 bit screen.
And this is a regression from bug 1209812, which removed gfxImageFormat::A1, which causes gfxImageFormatToCairoFormat to no longer work because gfxImageFormat and _cairo_format no longer match up.
Blocks: 1209812
Keywords: regression
This one's my fault.

In part 1 of bug 1209812 I made the conversions between gfxImageFormat and cairo_format_t more explicit -- they now use a macro instead of just casts, which is good.

But the macros still assume that the values in gfxImageFormat line up with the equivalent values in cairo_format_t. And then in part 2 I removed gfxImageFormat::A1, which breaks the assumption.

Should be an easy fix -- just make the values line up again by making them explicit. Patch coming up soon.
Markus, can you test this? Thank you.
Attachment #8683873 - Flags: review?(mstange)
Assignee: nobody → n.nethercote
Comment on attachment 8683873 [details] [diff] [review]
Re-establish equivalence between gfxImageFormat and cairo_format_t

Review of attachment 8683873 [details] [diff] [review]:

Attachment #8683873 - Flags: review?(mstange) → review+
It works!
Bug 1222171 - Re-establish equivalence between gfxImageFormat and cairo_format_t. r=mstange.
Thank you for the fast testing and review.
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
I have what seems to be the same issue on a released version of Firefox 44 -
It seems like this should've been uplifted to FF44?
Flags: needinfo?(n.nethercote)
Oops, pasted wrong bug #1243491 is the new report.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #11)
> It seems like this should've been uplifted to FF44?

Supplemental, Bryan confirmed over on bug 1243491 that the patch here fixes that bug. Unfortunately this missed the 44 release so we'd need to take this as a ride-along if we do a 44.0.1. Alternatively these folks will need to wait six weeks for 45.0 and switch to Beta in the meantime.
Comment on attachment 8683873 [details] [diff] [review]
Re-establish equivalence between gfxImageFormat and cairo_format_t

Approval Request Comment
[Feature/regressing bug #]: Bug 209812
[User impact if declined]: Crashes on remote desktop on linux
[Describe test coverage new/current, TreeHerder]: Fixed in 45 and comfirmed
[Risks and why]: Should be pretty low. This was just a logic error.
Attachment #8683873 - Flags: approval-mozilla-release?
I agree with Anthony in comment 14. Apologies for the failure to uplift.
Flags: needinfo?(n.nethercote)
Anthony, I might consider taking this fix in a 44.0.1. Before I can do that, can you confirm whether the fix here was verified or not? I know you said so in comment 14 but it doesn't hurt to check again. Thanks!
Flags: needinfo?(
We've verified it in Ubuntu's bug tracker here:

PPA direct link

We (I work for Canonical Support, btw) are currently pursuing an SRU (Stable Release Update) to get it to our users but doing 44.0.1 would be welcome!
Flags: needinfo?(
We are pondering a 44.0.1 dot release, can we get an uplift request here?
Flags: needinfo?(n.nethercote)
(In reply to Robert Kaiser ( from comment #20)
> We are pondering a 44.0.1 dot release, can we get an uplift request here?

KaiRo, the patch is already nominated for uplift to m-r in comment 15.
Flags: needinfo?(n.nethercote)
Comment on attachment 8683873 [details] [diff] [review]
Re-establish equivalence between gfxImageFormat and cairo_format_t

Given the verification mentioned in comment 19 and from the beta channel, this is a good & safe fix to take as a ride-along in 44.0.1
Attachment #8683873 - Flags: approval-mozilla-release? → approval-mozilla-release+
Bryan, could you please check that it works with 44.0.1 too? Thank you!
Flags: needinfo?(gQuigs+bugs)
I wasn't able to test (lost access to environment), but another user tested in the LP bug and confirmed it [1] worked.  Thanks!
Flags: needinfo?(gQuigs+bugs)
Thanks a lot, Bryan!

Marking as verified based on above comments.
Adding a release note for 44.0.1 as
Fix for graphics startup crash (Linux) with a link to the bug.
You need to log in before you can comment on or make changes to this bug.