Summary: | _cairo_color_compute_shorts fails with FPU set to single precision | ||
---|---|---|---|
Product: | cairo | Reporter: | Sebastian Tusk <freedesktop.20.scyt> |
Component: | general | Assignee: | Carl Worth <cworth> |
Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
Severity: | blocker | ||
Priority: | high | CC: | sam |
Version: | 1.2.1 | ||
Hardware: | x86 (IA32) | ||
OS: | Windows (All) | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Bug Depends on: | |||
Bug Blocks: | 8550 | ||
Attachments: |
proposed patch
correct patch |
Description
Sebastian Tusk
2006-07-12 01:22:57 UTC
Good catch. Can be committed IMO. I'd prefer this change to be discussed on the cairo mailing list before any change is made. We've had subtle bugs in this area before, so caution is called for. -Carl I confirmed that changing to 1e-2, the image surface still passes the test suite. *** Bug 7712 has been marked as a duplicate of this bug. *** The fix given does not help bug 7712, on sparc solaris. To fix that I did /* Try to recover a cairo_format_t from a pixman_format * by looking at the bpp and masks values. */ static cairo_format_t _cairo_format_from_pixman_format (pixman_format_t *pixman_format) { int bpp, am, rm, gm, bm; pixman_format_get_masks (pixman_format, &bpp, &am, &rm, &gm, &bm); /*fprintf(stderr, "[sws cairo] bpp %d\n", bpp);*/ switch (bpp) { case 32: if (am == 0xff000000 && rm == 0x00ff0000 && gm == 0x0000ff00 && bm == 0x000000ff) return CAIRO_FORMAT_ARGB32; if (am == 0x0 && rm == 0x00ff0000 && gm == 0x0000ff00 && bm == 0x000000ff) return CAIRO_FORMAT_RGB24; /* sws try this rather than crash */ fprintf(stderr, "[sws cairo] bpp %d, falling through!\n", bpp); return CAIRO_FORMAT_RGB24; I see the error printout here, but mozilla, firefox now work fine. Sam, I'm really sorry. I dupped to the wrong bug. It's fixed now. Please resubmit your comment to bug #7294. Carl, I raised this on the list: http://lists.freedesktop.org/archives/cairo/2006-July/007449.html with no response. Should we fix this now? (In reply to comment #7) > Carl, > > I raised this on the list: > > http://lists.freedesktop.org/archives/cairo/2006-July/007449.html > > with no response. Should we fix this now? Not yet. I've got a reply to make, but I need to draw some pictures first. -Carl Created attachment 6425 [details] [review] proposed patch Ok, using float.h to use the best epsilon possible. It uses DBL_EPSILON if it's defined, and 1e-7 otherwise. 1e-7 should work on 32bit float too. Can you check that this solves your problem? Lets not fix this in the 1.2.x. Created attachment 6428 [details] [review] correct patch Same patch, using floor() instead of relying of the FPU rounding down. This was extensively discussed here: http://lists.freedesktop.org/archives/cairo/2006-August/007709.html With proposed solution about here: http://lists.freedesktop.org/archives/cairo/2006-August/007718.html and an alternative solution here: http://lists.freedesktop.org/archives/cairo/2006-August/007732.html And finally, a dup here: http://lists.freedesktop.org/archives/cairo/2006-October/008181.html I think we can go with the first solution. I can push it in. Carl? (In reply to comment #12) > With proposed solution about here: > > http://lists.freedesktop.org/archives/cairo/2006-August/007718.html ... > I think we can go with the first solution. I can push it in. Carl? Behdad, Thanks for digging up all the old history on this---and sorry you had to do that. We had done all the work, but somehow I hadn't pushed out the branch, (I had one created with the correct fix sitting on it). I've pushed the fix out now: http://gitweb.freedesktop.org/?p=cairo;a=commit;h=b62710d4f8602203d848daf2d444865b611fff09 -Carl |
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.