Bug 34810

Summary:	[Bisected] many cairo test regressed
Product:	cairo	Reporter:	zhao jian <jian.j.zhao>
Component:	xlib backend	Assignee:	Chris Wilson <chris>
Status:	VERIFIED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	blocker
Priority:	high
Version:	1.10.3
Hardware:	x86 (IA32)
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:

Description zhao jian 2011-02-27 18:18:41 UTC

System Environment:
--------------------------
Libdrm:		(master)2.4.23-16-ge6018c25ca63fa6066d8fa6e57373030d07b0392
Mesa:		(master)a385ac62070fa68052c77df7be62685bf6a58992
Xserver:		(master)xorg-server-1.10.0
Xf86_video_intel: (master)2.14.0-38-g6b926444c7c66e5d14eb911d9ab3d0e2d512bca4
Cairo:		(master)17169a1e5130b8a287f07eb43d41f0b51307cc57
Kernel:	(drm-intel-next) 710f957846cff998c681f3701f6f90eda896458f


Bug detailed description:
-------------------------
There are many cairo test cases(about 40) regressed on xlib fallback backend with output: lt-cairo-test-suite: cairo-surface.c:178: _cairo_surface_set_error: Assertion `status < CAIRO_STATUS_LAST_STATUS\' failed. clear-source.xlib-fallback.rgb24 [0]: !!!CRASHED!!!
I find it was caused by commit badf32290ff894351e0f6879aafeac6db8e0d846 in Cairo. And this issue happened on both Pineview and Piketon. 
commit badf32290ff894351e0f6879aafeac6db8e0d846
Author: Benjamin Otte <otte@redhat.com>
Date:   Fri Feb 18 18:23:25 2011 +0100 
    surface: Don't be nice to people setting internal error codes 
    Just DIE DIE DIE in the _cairo_status_set_status() assertion.

Before this commit, its running result: 
[root@x-pk1 test]# CAIRO_TEST_TARGET=xlib ./cairo-test-suite bitmap-font
TESTING cairo-test-suite
Compiled against cairo 1.11.3, running on 1.11.3.
Compiled against pixman 0.21.5, running on 0.21.5.
TESTING bitmap-font
bitmap-font.xlib.argb32 [0]:    PASS
bitmap-font.xlib.rgb24 [0]:     PASS
bitmap-font.xlib-window.rgb24 [0]:      PASS
bitmap-font.xlib-fallback.rgb24 [0]:    PASS
bitmap-font: PASS
1 Passed, 0 Failed [0 crashed, 0 expected], 0 Skipped 

With this commit, its running result: 
[root@x-pk1 test]# CAIRO_TEST_TARGET=xlib ./cairo-test-suite bitmap-font
TESTING cairo-test-suite
Compiled against cairo 1.11.3, running on 1.11.3.
Compiled against pixman 0.21.5, running on 0.21.5.
TESTING bitmap-font
bitmap-font.xlib.argb32 [0]:    PASS
bitmap-font.xlib.rgb24 [0]:     PASS
bitmap-font.xlib-window.rgb24 [0]:      PASS
bitmap-font.xlib-fallback.rgb24 [0]:    lt-cairo-test-suite: cairo-surface.c:178: _cairo_surface_set_error: Assertion `status < CAIRO_STATUS_LAST_STATUS' failed.
bitmap-font.xlib-fallback.rgb24 [0]:    !!!CRASHED!!!
bitmap-font: CRASH! (xlib-fallback)
0 Passed, 1 Failed [1 crashed, 0 expected], 0 Skipped
xlib-fallback (rgb24): 1 crashed! - bitmap-font

I tested with the following patch which just partly revert that commit, it passed again. 

diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index b4c9eb6..74c2461 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -173,6 +173,9 @@ _cairo_surface_set_error (cairo_surface_t *surface,
         status == CAIRO_INT_STATUS_NOTHING_TO_DO)
         return CAIRO_STATUS_SUCCESS;
 
+    if (status >= CAIRO_INT_STATUS_UNSUPPORTED)
+        return status;
+
     /* Don't overwrite an existing error. This preserves the first
      * error, which is the most significant. */
     _cairo_status_set_error (&surface->status, status);

Comment 1 Benjamin Otte 2011-02-27 18:36:32 UTC

Yeah, that was me.

While investigating what to do about cairo_status_t vs cairo_int_status_t, I realized that functions may well return internal status code indicating errors from their vfuncs and we just ignored those errors. And that is not a good idea. So to be sure this gets fixed, I made sure it's broken in a very noticable way.

Now, what makes this very complicated is the fact that the analysis surface actually thinks it's perfectly ok to return internal error codes as the result of its analysis, so things break when the analysis surface participates in the test.

I have no idea how to best handle this - and neither did anybody I talked to on IRC, but it needs to fixed and fixed correctly before we release another cairo version

Comment 2 zhao jian 2011-03-07 18:18:40 UTC

It works well with then newest cairo on master branch. 

Tested with: 
Libdrm:		(master)2.4.24-6-g3b04c73650b5e9bbcb602fdb8cea0b16ad82d0c0
Mesa:		(master)6547253bd138db815173c00ca2dc220e8ad20ab1
Xserver:		(master)xorg-server-1.10.0-62-g628d16a92a7fa556fbb70bf4a4adf57ec05c190b
Xf86_video_intel:		(master)2.14.901-8-gb06f0f1531fd49309c66ffbe87d6476d3271b5df
Cairo:		(master)f1d313e042af89b2f5f5d09d3eb1703d0517ecd7
Kernel:	(drm-intel-next) 47ae63e0c2e5fdb582d471dc906eb29be94c732f

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.