| Summary: | Cairo Quartz should be using CGFloat | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Kristian Rietveld <kris> |
| Component: | quartz backend | Assignee: | Vladimir Vukicevic <vladimir> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | kris, pavel, ranma42 |
| Version: | 1.9.5 | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | [quartz] Define cairo_quartz_float_t and use instead of float | ||
If CGFloat isn't available without the 10.6 SDK, then I'd suggest that we don't use it -- instead, we should have a cairo typedef, cairo_quartz_float_t or something similar that's a float on 32-bit and a double on 64-bit builds. (Could also typedef CGFloat if it isn't present in the current SDK version -- always to float -- but that might be fragile.) (In reply to comment #1) > If CGFloat isn't available without the 10.6 SDK, then I'd suggest that we don't > use it -- instead, we should have a cairo typedef, cairo_quartz_float_t or > something similar that's a float on 32-bit and a double on 64-bit builds. > (Could also typedef CGFloat if it isn't present in the current SDK version -- > always to float -- but that might be fragile.) That would be easiest, since it will save you the hassle of detecting against which system libraries you are building (which might be fragile as well). A possible construction could be (that has to be placed after inclusion of the system headers): #ifdef CGFLOAT_DEFINED typedef CGFloat cairo_quartz_float_t; #else typedef float cairo_quartz_float_t; #endif The system headers define CGFLOAT_DEFINED right after the CGFloat typedef and otherwise the define will not be there. To me this seems something we can safely rely on. Created attachment 32340 [details] [review] [quartz] Define cairo_quartz_float_t and use instead of float This is a patch replacing all usages of float with cairo_quartz_float_t. I have not touched the current usages of double, because when done these would become floats on 32-bit Mac builds, which is probably not wished. I'm having strange issues with Cairo Quartz backend on 64bit Snow Leopard (it works perfect on 32bit). Basically gradients/patterns are not working at all. I think this bug might be causing the problem so I will give that patch a test and let you know if this is the case. Screenshots: 1) Without Quartz on 64bit or with Quartz on 32bit http://img24.imageshack.us/img24/8039/screenshot20100222at222.png 2) With Quartz on 64bit http://img28.imageshack.us/img28/8039/screenshot20100222at222.png You can see basic shapes are working ok, except gradient fills. (In reply to comment #4) > I'm having strange issues with Cairo Quartz backend on 64bit Snow Leopard (it > works perfect on 32bit). Basically gradients/patterns are not working at all. I > think this bug might be causing the problem so I will give that patch a test > and let you know if this is the case. I think the patch I posted in comment 3 will fix this. It also touches the gradient parts of the cairo code. Have you given the patch a test yet? Does it resolve the problems for you? Hi, I'm sorry for delayed reply, I was busy with some other projects. Now I returned to my project and tried the patch against Cairo 1.9.6 snapshot, and I can VERIFY this patch will fix the problems and gradients now work as expected! I tried on 32bit and 64bit as well and it works very well ... thanks! (In reply to comment #6) > Now I returned to my project and tried the patch against Cairo 1.9.6 snapshot, > and I can VERIFY this patch will fix the problems and gradients now work as > expected! I tried on 32bit and 64bit as well and it works very well ... thanks! Thanks for verifying the patch. I do not think it has been committed yet to the Cairo git repository, so I am reopening the bug. Could one of the Cairo maintainers review/approve and commit this patch? It would be great to have this in. I'm now playing a little bit more with cairo snapshot 1.9.6 and Quartz surfaces. Sometimes I'm using CGContextSetShadow and then paint something with Cairo and it is supposed to have shadows. It works correctly on Leopard 10.5 and Cairo 1.8.10, but here for x86_64 and Snow Leopard (cairo 1.9.6) it doesn't work correctly and x-offset of the shadow seems to be always shifted by 20 pixels to the right. Do you think this could have something to do with some CGFloat declarations? (In reply to comment #8) > I'm now playing a little bit more with cairo snapshot 1.9.6 and Quartz > surfaces. Sometimes I'm using CGContextSetShadow and then paint something with > Cairo and it is supposed to have shadows. > > It works correctly on Leopard 10.5 and Cairo 1.8.10, but here for x86_64 and > Snow Leopard (cairo 1.9.6) it doesn't work correctly and x-offset of the shadow > seems to be always shifted by 20 pixels to the right. Do you think this could > have something to do with some CGFloat declarations? I'm not sure about it, but I don't expect it to be related to this bug. Anyway, git master now contains a fix, with just a minor change from Kristian Rietveld's patch attached here. Sorry for the delay in pushing the patch upstream (I was not in CC, so I didn't notice that it had been successfully tested). Special thanks to Kristian and Pavel. |
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.
Since Mac OS 10.6, CoreGraphics has switched to using CGFloat. CGFloat is defined to be a float on 32-bit systems and a double on 64-bit systems. Currently the Cairo Quartz backend uses plain floats, which causes issues with 64-bit builds. For example, calls to CGContextSetLineDash() pass an array of floats now, where an array of doubles is expected on 64-bit. This particular case breaks GTK+'s drawing of the dashed focus rectangles on 64-bit Mac OS. Now that CGFloat is only available on Mac OS 10.6 and higher, we have to proceed with caution. Something likewise to "#ifdef NSINTEGER_DEFINED" can likely be done, if CGFloat has not been defined by system headers Cairo can define it to be float by default. Then, in all of the cairo-quartz-surface.c source code CGFloat should be used. An example that fixes the dashed line case for me: diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c index 44b25a3..a48e6d3 100644 --- a/src/cairo-quartz-surface.c +++ b/src/cairo-quartz-surface.c @@ -2039,20 +2039,20 @@ _cairo_quartz_surface_stroke (void *abstract_surface, if (style->dash && style->num_dashes) { #define STATIC_DASH 32 - float sdash[STATIC_DASH]; - float *fdash = sdash; + CGFloat sdash[STATIC_DASH]; + CGFloat *fdash = sdash; unsigned int max_dashes = style->num_dashes; unsigned int k; if (style->num_dashes%2) max_dashes *= 2; if (max_dashes > STATIC_DASH) - fdash = _cairo_malloc_ab (max_dashes, sizeof (float)); + fdash = _cairo_malloc_ab (max_dashes, sizeof (CGFloat)); if (fdash == NULL) return _cairo_error (CAIRO_STATUS_NO_MEMORY); for (k = 0; k < max_dashes; k++) - fdash[k] = (float) style->dash[k % style->num_dashes]; + fdash[k] = (CGFloat) style->dash[k % style->num_dashes]; CGContextSetLineDash (surface->cgContext, style->dash_offset, fdash, max if (fdash != sdash) If wished I can work on a patch that fully migrates the Cairo Quartz backend to CGFloat and test this on an old school 32-bit Tiger machine and a new school 64-bit machine.