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

VERIFIED FIXED in Firefox 44

Status

()

defect
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: mstange, Assigned: njn)

Tracking

({regression})

Trunk
mozilla45
All
Linux
Points:
---

Firefox Tracking Flags

(firefox44+ verified, firefox45 verified, relnote-firefox 44+)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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".

Stack:
> #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.
(Reporter)

Comment 1

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

Comment 2

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

Comment 3

4 years ago
Markus, can you test this? Thank you.
Attachment #8683873 - Flags: review?(mstange)
(Assignee)

Updated

4 years ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
(Reporter)

Comment 4

4 years ago
Comment on attachment 8683873 [details] [diff] [review]
Re-establish equivalence between gfxImageFormat and cairo_format_t

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

Sure.
Attachment #8683873 - Flags: review?(mstange) → review+
(Reporter)

Comment 5

4 years ago
It works!
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2da18c113b3722ea61e51077ba7dd31e547a0fee
Bug 1222171 - Re-establish equivalence between gfxImageFormat and cairo_format_t. r=mstange.
(Assignee)

Comment 7

4 years ago
Thank you for the fast testing and review.

Comment 8

4 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2da18c113b37
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

Comment 10

3 years ago
I have what seems to be the same issue on a released version of Firefox 44 - https://bugzilla.mozilla.org/show_bug.cgi?id=1222171
It seems like this should've been uplifted to FF44?
Flags: needinfo?(n.nethercote)

Comment 12

3 years ago
Oops, pasted wrong bug #1243491 is the new report.
Duplicate of this bug: 1243491
(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?
(Assignee)

Comment 16

3 years ago
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?(anthony.s.hughes)

Comment 19

3 years ago
We've verified it in Ubuntu's bug tracker here:
https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1538724

PPA direct link 
https://launchpad.net/~dgadomski/+archive/ubuntu/lp1538724

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?(anthony.s.hughes)

Comment 20

3 years ago
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 (:kairo@mozilla.com) 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)

Comment 25

3 years ago
I wasn't able to test (lost access to environment), but another user tested in the LP bug and confirmed it [1] worked.  Thanks!
[1] https://ftp.mozilla.org/pub/firefox/candidates/44.0.1-candidates/build1/linux-x86_64/en-US/
Flags: needinfo?(gQuigs+bugs)
Thanks a lot, Bryan!

Marking as verified based on above comments.
Status: RESOLVED → VERIFIED
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.