Bug 7229 - assertion failure in _cairo_content_from_format on 16-bit X server
assertion failure in _cairo_content_from_format on 16-bit X server
Status: RESOLVED FIXED
Product: cairo
Classification: Unclassified
Component: general
1.1.8
x86 (IA32) Linux (All)
: high normal
Assigned To: Carl Worth
cairo-bugs mailing list
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-14 10:37 UTC by Frederic Crozat
Modified: 2006-06-16 09:31 UTC (History)
0 users

See Also:


Attachments
Add support for CAIRO_FORMAT_RGB16_565 to cairo 1.1.8 (8.00 KB, patch)
2006-06-15 16:58 UTC, Carl Worth
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Crozat 2006-06-14 10:37:14 UTC
a regression has been introduced by d0dd3b822e98358e88f8c9261ef633331548ccfd ( 
New API: Add new function cairo_surface_get_content ).

Try running any gtk application in an Xnest (Xorg 7.1) and you'll get an
assertion  with the following stacktrace :

#0  0xbfffe410 in __kernel_vsyscall ()

#1  0xb75b0fa1 in raise () from /lib/i686/libc.so.6

#2  0xb75b2868 in abort () from /lib/i686/libc.so.6

#3  0xb75aa53c in __assert_fail () at hashval.h:43

#4  0xb780582b in _cairo_content_from_format (format=4294967295)

    at cairo-image-surface.c:471

#5  0xb7805046 in _cairo_image_surface_create_for_pixman_image (

    pixman_image=0x827eeb8, format=4294967295) at cairo-image-surface.c:66

#6  0xb7805308 in _cairo_image_surface_create_with_masks (

    data=0xb6443008
"<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç<ç"...,

    format=0xbff746b8, width=361, height=208, stride=724)

    at cairo-image-surface.c:159

#7  0xb782f95d in _get_image_surface (surface=0x827e910,

    interest_rect=0xbff747dc, image_out=0xbff7471c, image_rect=0xbff747e8)

    at cairo-xlib-surface.c:663

#8  0xb782fd7e in _cairo_xlib_surface_acquire_dest_image (

    abstract_surface=0x827e910, interest_rect=0xbff747dc,

    image_out=0xbff747e4, image_rect_out=0xbff747e8, image_extra=0xbff747f0)

    at cairo-xlib-surface.c:821

#9  0xb780f987 in _cairo_surface_acquire_dest_image (surface=0x827e910,

    interest_rect=0xbff747dc, image_out=0xbff747e4, image_rect=0xbff747e8,

---Type <return> to continue, or q <return> to quit---

    image_extra=0xbff747f0) at cairo-surface.c:845

#10 0xb78115b2 in _fallback_init (state=0xbff747d8, dst=0x827e910, x=0, y=0,

    width=361, height=208) at cairo-surface-fallback.c:76

#11 0xb7813012 in _cairo_surface_fallback_fill_rectangles (surface=0x827e910,

    op=CAIRO_OPERATOR_OVER, color=0xbff74a30, rects=0x827e608, num_rects=1)

    at cairo-surface-fallback.c:1078

#12 0xb780ff81 in _cairo_surface_fill_rectangles (surface=0x827e910,

    op=CAIRO_OPERATOR_OVER, color=0xbff74a30, rects=0x827e608, num_rects=1)

    at cairo-surface.c:1145

#13 0xb780fe94 in _cairo_surface_fill_region (surface=0x827e910,

    op=CAIRO_OPERATOR_OVER, color=0xbff74a30, region=0x827e5f8)

    at cairo-surface.c:1094

#14 0xb78122e6 in _clip_and_composite_trapezoids (src=0xbff749ec,

    op=CAIRO_OPERATOR_OVER, dst=0x827e910, traps=0xbff74968, clip=0x827eaac,

    antialias=CAIRO_ANTIALIAS_DEFAULT) at cairo-surface-fallback.c:597

#15 0xb78128d2 in _cairo_surface_fallback_fill (surface=0x827e910,

    op=CAIRO_OPERATOR_OVER, source=0xbff749ec, path=0x827e9f8,

    fill_rule=CAIRO_FILL_RULE_WINDING, tolerance=0.10000000000000001,

    antialias=CAIRO_ANTIALIAS_DEFAULT) at cairo-surface-fallback.c:832

#16 0xb781048d in _cairo_surface_fill (surface=0x827e910,

    op=CAIRO_OPERATOR_OVER, source=0xbff74aa0, path=0x827e9f8,

    fill_rule=CAIRO_FILL_RULE_WINDING, tolerance=0.10000000000000001,

    antialias=CAIRO_ANTIALIAS_DEFAULT) at cairo-surface.c:1300

---Type <return> to continue, or q <return> to quit---

#17 0xb78035ec in _cairo_gstate_fill (gstate=0x827ea28, path=0x827e9f8)

    at cairo-gstate.c:972

#18 0xb77fde30 in *INT_cairo_fill_preserve (cr=0x827e9f0) at cairo.c:1848

#19 0xb77fddf7 in cairo_fill (cr=0x827e9f0) at cairo.c:1826

#20 0xb7939145 in gdk_window_clear_backing_rect (window=0x80730c8, x=0, y=0,

    width=361, height=208) at gdk/gdkwindow.c:1877

#21 0xb7939a4d in IA__gdk_window_begin_paint_region (window=0x80730c8,

    region=0x80c4140) at gdk/gdkwindow.c:1016

#22 0xb7af2a43 in IA__gtk_main_do_event (event=0xbff74c60)

    at gtk/gtkmain.c:1377

#23 0xb7939e75 in gdk_window_process_updates_internal (window=0x80730c8)

    at gdk/gdkwindow.c:2324

#24 0xb793a0bc in IA__gdk_window_process_all_updates () at gdk/gdkwindow.c:2387

#25 0xb793a132 in gdk_window_update_idle (data=0x0) at gdk/gdkwindow.c:2245

#26 0xb773b4c0 in g_idle_dispatch (source=0x827e108,

    callback=0xb793a110 <gdk_window_update_idle>, user_data=0x0)

    at gmain.c:3924

#27 0xb773d295 in IA__g_main_context_dispatch (context=0x8075490)

    at gmain.c:2043

#28 0xb77402b2 in g_main_context_iterate (context=0x8075490, block=1,

    dispatch=1, self=0x8076650) at gmain.c:2675

#29 0xb7740624 in IA__g_main_loop_run (loop=0x825cbd8) at gmain.c:2879

#30 0xb7af2caf in IA__gtk_main () at gtk/gtkmain.c:999

---Type <return> to continue, or q <return> to quit---

#31 0x0804d66c in ?? ()

#32 0xb759e728 in __libc_start_main (main=0x804d4b0, argc=2,

    ubp_av=0xbff74f24, init=0x8054fd0, fini=0x8054fc0,

    rtld_fini=0xb7f702f0 <_dl_fini>, stack_end=0xbff74f1c) at libc-start.c:231

#33 0x0804d421 in ?? ()
Comment 1 Carl Worth 2006-06-14 11:46:40 UTC
(In reply to comment #0)
> Try running any gtk application in an Xnest (Xorg 7.1) and you'll get an
> assertion  with the following stacktrace :

I tried that but didn't have any luck replicating the bug. What depth is your
Xnest running at?

There's a particular part of the stack trace that looks bad:

> #6  0xb7805308 in _cairo_image_surface_create_with_masks ... at
cairo-image-surface.c:159
> #7  0xb782f95d in _get_image_surface ... at cairo-xlib-surface.c:663

That looks like we're hitting the code path in cairo-xlib-surface.c marked as:

        /*
         * XXX This can't work.  We must convert the data to one of the
         * supported pixman formats.  Pixman needs another function
         * which takes data in an arbitrary format and converts it
         * to something supported by that library.
         */

which is about as discouraging as a comment could be.

But I don't yet see anything in the commit of interest that would obviously
cause it to start hitting this path.

-Carl
 
Comment 2 Frederic Crozat 2006-06-15 00:46:59 UTC
I think we are hitting the bug because Xnest is missing render extension.

I don't have any problem when using Xephyr, unless I disable Render extension in
Xephyr.

I'm running at 16bpp

xdpyinfo on Xnest returns :
name of display:    :2.0
version number:    11.0
vendor string:    The X.Org Foundation
vendor release number:    70100000
X.Org version: 7.1.0
maximum request size:  16777212 bytes
motion buffer size:  256
bitmap unit, bit order, padding:    32, LSBFirst, 32
image byte order:    LSBFirst
number of supported pixmap formats:    7
supported pixmap formats:
    depth 1, bits_per_pixel 1, scanline_pad 32
    depth 4, bits_per_pixel 8, scanline_pad 32
    depth 8, bits_per_pixel 8, scanline_pad 32
    depth 15, bits_per_pixel 16, scanline_pad 32
    depth 16, bits_per_pixel 16, scanline_pad 32
    depth 24, bits_per_pixel 32, scanline_pad 32
    depth 32, bits_per_pixel 32, scanline_pad 32
keycode range:    minimum 8, maximum 255
focus:  PointerRoot
number of extensions:    23
    BIG-REQUESTS
    DAMAGE
    DEC-XTRAP
    DOUBLE-BUFFER
    Extended-Visual-Information
    GLX
    LBX
    MIT-SCREEN-SAVER
    MIT-SUNDRY-NONSTANDARD
    Multi-Buffering
    RECORD
    SECURITY
    SGI-GLX
    SHAPE
    SYNC
    TOG-CUP
    X-Resource
    XC-APPGROUP
    XC-MISC
    XFIXES
    XFree86-Bigfont
    XTEST
    XVideo
default screen number:    0
number of screens:    1

screen #0:
  print screen:    no
  dimensions:    960x768 pixels (256x212 millimeters)
  resolution:    95x92 dots per inch
  depths (2):    1, 16
  root window id:    0x28
  depth of root window:    16 planes
  number of colormaps:    minimum 1, maximum 1
  default colormap:    0x25
  default number of colormap cells:    64
  preallocated pixels:    black 0, white 65535
  options:    backing-store NO, save-unders NO
  largest cursor:    64x64
  current input event mask:    0x0
  number of visuals:    4
  default visual id:  0x21
  visual:
    visual id:    0x21
    class:    TrueColor
    depth:    16 planes
    available colormap entries:    64 per subfield
    red, green, blue masks:    0xf800, 0x7e0, 0x1f
    significant bits in color specification:    8 bits

Comment 3 Frederic Crozat 2006-06-15 01:01:44 UTC
Ok, I've tested with 24bpp and cairo doesn't assert in Xnest.

So, it is a combination of Xnest + 16bpp 
Comment 4 Carl Worth 2006-06-15 07:58:35 UTC
OK. So we're looking for a regresion related to 16bpp between 1.1.6 and 1.1.8,
right?

That leads me to think the bug might be here:

http://gitweb.cairographics.org/?p=cairo;a=commit;h=ea05e027111d5f336b7e3f2170f929b0b1e37692

Can you try reverting this and see if the bug goes away? That should be as
simple as running the following from a git clone of cairo:

git revert ea05e027111d5f336b7e3f2170f929b0b1e37692

(I just ran that and noticed in conflicts in src/cairo-xlib-surface.c, but it
appears that it's a trivial conflict---just resolve by moving the conflict
markers I think.)

-Carl
Comment 5 Carl Worth 2006-06-15 12:24:30 UTC
Renaming bug to indicate dependence on 16-bit X server.

-Carl
Comment 6 Carl Worth 2006-06-15 12:50:26 UTC
I've reproduced now with the following two-liner (thanks to Jeremy Katz for the
suggestion).

Xvnc -depth 16 SecurityTypes=None -auth /dev/null :8 &
DISPLAY=:8 python -c 'import gtk; w = gtk.Window(); w.show(); gtk.main()'

I've also verified that my guess at the bogus commit was wrong. I reverted
ea05e0271 but still got the failure. That's really not _too_ surprising since I
should have remembered that it was git-bisect that pointed to d0dd3b822 as the
problem. I'll look closer now.

-Carl
Comment 7 Carl Worth 2006-06-15 14:27:33 UTC
I understand the current behavior much better now.

It appears that we've always been generating a bogus image surface internal to
cairo every time we were running on an X server with a native visual that
doesn't match one of cairo's standard image formats, (currently ARGB32, RGB24,
A8, and A1). The bogus surface always had an invalid format value of -1.

The commit pointed to by the bisect is the first time there's been code that
actually looks at the invalid format value.

In some sense, the bogus value can be ignored (as was happening with older cairo
versions) but this is a really unhealthy situation, particularly as of cairo
1.1.8 where the user has functions for extracting the data from an image surface
(cairo_image_surface_get_data/format/width/height/stride). Those won't work with
a -1 for format.

What I've done now is push out a change to cairo that moves the assertion up
earlier to where the -1 value starts from. It now also prints a more helpful
error message. For example, with Xvnc -depth 16 I now get:

Error: Cairo does not yet support the requested image format:
        Depth: 16
        Alpha mask: 0x00000000
        Red   mask: 0x0000f800
        Blue  mask: 0x000007e0
        Green mask: 0x0000001f
Please file a enhacement request (quoting the above) at:
http://bugs.freedesktop.org/enter_bug.cgi?product=cairo
python: cairo-image-surface.c:130: _cairo_format_from_pixman_format: Assertion
`NOT_REACHED' failed.

I would be interested to know if anyone hitting this bug is getting anything
different than that.

I think it would be quite easy to simply add direct support to cairo for a
16-bit 565 image format which would solve this bug properly, (at least for X
servers with the image format masks shown above). So let me know if you get
anything different.

I'll go float the 565 image format idea over on the cairo list.

-Carl
Comment 8 Carl Worth 2006-06-15 16:58:40 UTC
Created attachment 5923 [details] [review]
Add support for CAIRO_FORMAT_RGB16_565 to cairo 1.1.8

I think this is the patch we'll want to fix this, (generated so that it should
apply fine to 1.1.8).

This patch represents three tiny commits that are already in cairo upstream
(after 1.1.8) and one additonal patch that has been proposed on the cairo
mailing list to add CAIRO_FORMAT_RGB16_565.

This approach does add new, public API so it's maybe not desired for something
like a distribution-specific patch, (though the same basic patch that just
defined CAIRO_FORMAT_RGB16_565 with some huge number inside cairoint.h might be
just fine).

Another option is that if this works alright, and isn't rejected by any
upstream cairo developers, then I can just make a new 1.1.10 snapshot right
away.

Anyway, please test this and let me know how it works for you.

-Carl
Comment 9 Frederic Crozat 2006-06-16 03:02:15 UTC
I can confirm adding support for RGB16_565 fixes the assertion I'm seeing.

I strongly suggest adding this feature to cairo for 1.1.10, since you can be
sure we will see complains from VNC users or closed source X terminal otherwise ;)
Comment 10 Carl Worth 2006-06-16 09:31:36 UTC
Thanks for confirming this fix.

I've pushed the last piece of this out to cairo now:

http://gitweb.cairographics.org/?p=cairo;a=commit;h=c6164d0d2a9ca16a00c2768f24a0230de6cbc6f6

and I'll now push out a 1.1.10 snapshot including this.

-Carl