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"
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla45
People
(Reporter: mstange, Assigned: n.nethercote)
References
Details
(Keywords: regression)
Attachments
(1 file)
2.07 KB,
patch
|
mstange
:
review+
ritu
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
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•9 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•9 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•9 years ago
|
||
Markus, can you test this? Thank you.
Attachment #8683873 -
Flags: review?(mstange)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•9 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•9 years ago
|
||
It works!
Assignee | ||
Comment 6•9 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•9 years ago
|
||
Thank you for the fast testing and review.
Comment 8•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Reporter | ||
Comment 9•9 years ago
|
||
Comment 10•9 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
Comment 11•9 years ago
|
||
It seems like this should've been uplifted to FF44?
Flags: needinfo?(n.nethercote)
Comment 12•9 years ago
|
||
Oops, pasted wrong bug #1243491 is the new report.
Comment 14•9 years ago
|
||
(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 15•9 years ago
|
||
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•9 years ago
|
||
I agree with Anthony in comment 14. Apologies for the failure to uplift.
Flags: needinfo?(n.nethercote)
See bug 916762 comment 4 which came back up in 44.
Updated•9 years ago
|
status-firefox44:
--- → affected
tracking-firefox44:
--- → ?
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•9 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•9 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+
Comment 23•9 years ago
|
||
bugherder uplift |
Comment 24•9 years ago
|
||
Bryan, could you please check that it works with 44.0.1 too? Thank you!
Flags: needinfo?(gQuigs+bugs)
Comment 25•9 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)
Comment 26•9 years ago
|
||
Thanks a lot, Bryan!
Marking as verified based on above comments.
Comment 27•9 years ago
|
||
Adding a release note for 44.0.1 as
Fix for graphics startup crash (Linux) with a link to the bug.
relnote-firefox:
--- → 44+
You need to log in
before you can comment on or make changes to this bug.
Description
•