| Summary: | 1.8.0 Cairo::Matrix API/ABI breakage | ||
|---|---|---|---|
| Product: | cairomm | Reporter: | Jānis Rukšāns <thedogfarted> |
| Component: | General | Assignee: | Murray Cumming <murrayc> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | medium | CC: | jonathon |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Revert "scaled_matrix() -> scaling_matrix()"
Revert "Add an overloaded Context::get_matrix() that returns a copy of the Matrix" Revert "Fix broken Pattern::get/set_matrix() API that was using the C types" Revert "Overhaul of the newly-wrapped Matrix API (+documentation)" Revert "Inherit Cairo::Matrix from cairo_matrix_t" Fix test breakage from reverting Matrix changes Add overloads taking (const) cairo_matrix_t & to keep ABI |
||
This is extremely unfortunate. I seem to have really screwed this up. I guess I'll have to revert all of those matrix changes. I naively thought they would be OK for API/ABI... I think you should first establish what you thought would happen and what has actually happened. Maybe you can then make only a limited change to fix it. We maybe also need to consider if this new ABI has already become established. What has happened is the mangled function names were changed. For functions that take reference to Cairo::Matrix as argument, overloads taking a reference to cairo_matrix_t should fix forward ABI compatibility. Backwards compatibility would still be broken and code requiring 1.6.x but built with 1.8.x headers won't run with 1.6.x at runtime. ATM I can't think of anything that could fix the latter. Can't we just add back a Context::set_matrix() overload that takes cairo_matrix_t? Well you need overloads for every function that takes Cairo::Matrix. And if you don't care about code compiled with 1.8.x headers but running with 1.6.x then that should be it (sorry I messed up with backward/forward compatibility). Created attachment 26572 [details] [review] Revert "scaled_matrix() -> scaling_matrix()" This reverts commit 87a5ddbd0f2709bdeb1d12634a37ad762adc44ab. Conflicts: ChangeLog Created attachment 26573 [details] [review] Revert "Add an overloaded Context::get_matrix() that returns a copy of the Matrix" This reverts commit ccb381d30a1a523896d2b60204c0c2588fe25f02. Created attachment 26574 [details] [review] Revert "Fix broken Pattern::get/set_matrix() API that was using the C types" This reverts commit bbf5159397aac3f952d0c17a5e66e7fc40177b8b. Conflicts: ChangeLog cairomm/pattern.h Created attachment 26575 [details] [review] Revert "Overhaul of the newly-wrapped Matrix API (+documentation)" This reverts commit dc26aaf781da09b69598cf106ec79c8c06fca8fa. Conflicts: .gitignore ChangeLog cairomm/matrix.h tests/test-font-face.cc tests/test-matrix.cc Created attachment 26576 [details] [review] Revert "Inherit Cairo::Matrix from cairo_matrix_t" This reverts commit c0fab1927cab33f35d43b421c3464351bb8d8072. Conflicts: cairomm/matrix.h cairomm/types.h tests/Makefile.am tests/test-font-face.cc tests/test-matrix.cc Created attachment 26577 [details] [review] Fix test breakage from reverting Matrix changes I've attached a set of patches that revert all of the matrix changes that caused this whole mess. Should we push these and declare 1.8.0 unusably broken and make a new release? I really think you should consider if both ABIs can be supported. I agree with Murray. I patched and rebuilt F10's cairomm-1.8.0 RPMs with overloads taking (const) cairo_matrix_t & and BMPx runs fine. Is there anything else that I could test that uses other functions than Context::set_matrix? Created attachment 26580 [details] [review] Add overloads taking (const) cairo_matrix_t & to keep ABI I added the overloads only to pre-1.8.0 functions. They're public but I think they might as well be private. hm, I seem to have forgotten to close this bug. I had committed patch #26580 quite some time ago, so it has been fixed since release 1.8.2. Thanks for the patch, and sorry for the lack of feedback. |
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.
* Cairo::Matrix now inherits from cairo_matrix_t * Previously, we had used Cairo::Matrix throughout our API, but Cairo::Matrix was just a typedef for cairo_matrix_t This breaks cairomm API/ABI. For example, BMPx (built against 1.6.x, uses cairomm to display "libnotify-like" popup for tray icon) crashes with /usr/libexec/beep-media-player-2-bin: undefined symbol: _ZN5Cairo7Context10set_matrixERK13_cairo_matrix In a way, API is also broken and rebuilding doesn't help: popup.cc: In member function 'void Bmp::Popup::draw_arrow(Cairo::RefPtr<Cairo::Context>&, int, int)': popup.cc:311: error: braces around initializer for non-aggregate type 'Cairo::Matrix' The offending line is Cairo::Matrix matrix = { 1, 0, 0, -1, 0, h };