New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various cleanups and bugfixes #207

Merged
merged 19 commits into from Sep 8, 2018

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Sep 8, 2018

This, among other things, fixes the spurious crash on CI in the testsuite and various other memory safety problems.

Also simplifies some code and adds various trait impls.

See individual commits.

sdroege added some commits Sep 8, 2018

Get rid of the CVec dependency and simply the PathSegment iterator
Especially use the correct types and make use of Rust's union support
for the PathSegment iterator instead of transmuting between types and
hoping that it plays out fine.
Don't transmute Boxes but instead just pass references to stack varia…
…bles

Apart from preventing some useless heap allocations, this also fixes
memory leaks in a few cases where the Boxes were actually leaked.

In addition, also return the radiuses of the circles in
RadialGradient::get_radial_circles()
Enable Context::push_group_with_content()
It was commented out but fully functional.
Remove custom CallbackGuard
Unwinding via FFI is not undefined behaviour anymore but panics
directly.
Uncomment Region::create_rectangles()
It was commented out and incorrect, now it should be fine.
Add SurfaceExt::status()
So it can be used on all surfaces and not only the base class
Make SurfacePriv trait pub(crate)
It's private but used in another module here.
Rename Path::get_ptr() and ::wrap() to ::as_ptr() and ::from_raw_full…
…() for consistency

And also make ::from_raw_full() unsafe because that's what it is.
Rewrite how refcounting and pointer conversions work for Pattern
It was unsafe before as it could "borrow" the pointer owned by a
context without ensuring that the context actually stays in scope and
had various functions that (unsafely without marked as such) change if
and how the Pattern would be dropped or not.
This caused spurious crashes in the testsuite.

Also rename pointer functions to be more consistent with how they are
called elsewhere and implement Clone and Debug for the patterns.
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Sep 8, 2018

Member

Gdk need fixed too, or removing wrap need postponed in Pattern

error[E0599]: no variant named `wrap` found for type `cairo::Pattern` in the current scope
   --> C:\Users\appveyor\.cargo\git\checkouts\gdk-0e2106e2e12a0cc1\c7ae4cf\src\window.rs:198:22
    |
198 |                 Some(cairo::Pattern::wrap(ret))
    |                      ^^^^^^^^^^^^^^^^^^^^ variant not found in `cairo::Pattern`
error[E0599]: no method named `get_ptr` found for type `&cairo::Pattern` in the current scope
   --> C:\Users\appveyor\.cargo\git\checkouts\gdk-0e2106e2e12a0cc1\c7ae4cf\src\window.rs:207:25
    |
207 |                 pattern.get_ptr()
    |                         ^^^^^^^
Member

EPashkin commented Sep 8, 2018

Gdk need fixed too, or removing wrap need postponed in Pattern

error[E0599]: no variant named `wrap` found for type `cairo::Pattern` in the current scope
   --> C:\Users\appveyor\.cargo\git\checkouts\gdk-0e2106e2e12a0cc1\c7ae4cf\src\window.rs:198:22
    |
198 |                 Some(cairo::Pattern::wrap(ret))
    |                      ^^^^^^^^^^^^^^^^^^^^ variant not found in `cairo::Pattern`
error[E0599]: no method named `get_ptr` found for type `&cairo::Pattern` in the current scope
   --> C:\Users\appveyor\.cargo\git\checkouts\gdk-0e2106e2e12a0cc1\c7ae4cf\src\window.rs:207:25
    |
207 |                 pattern.get_ptr()
    |                         ^^^^^^^
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege
Member

sdroege commented Sep 8, 2018

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Sep 8, 2018

Member

@sdroege Can you add other fix to cairo-sys-rs\Cargo.toml after gtk-rs/gdk#243 merged:
change 3 your optional dependencies to multiline format: [dependencies.glib-sys] etc.

Member

EPashkin commented Sep 8, 2018

@sdroege Can you add other fix to cairo-sys-rs\Cargo.toml after gtk-rs/gdk#243 merged:
change 3 your optional dependencies to multiline format: [dependencies.glib-sys] etc.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 8, 2018

Member

Will do

Member

sdroege commented Sep 8, 2018

Will do

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Sep 8, 2018

Member

@sdroege Other that noted issues all look good for me, thanks

Member

EPashkin commented Sep 8, 2018

@sdroege Other that noted issues all look good for me, thanks

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 8, 2018

Member

Thanks for the review :)

Member

sdroege commented Sep 8, 2018

Thanks for the review :)

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez
Member

GuillaumeGomez commented Sep 8, 2018

👍

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 8, 2018

Member

macOS build failure should be fixed now

Member

sdroege commented Sep 8, 2018

macOS build failure should be fixed now

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Sep 8, 2018

Member

@sdroege Opps, you already fixed it ;)

Member

EPashkin commented Sep 8, 2018

@sdroege Opps, you already fixed it ;)

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 8, 2018

Member

@GuillaumeGomez all green :)

Member

sdroege commented Sep 8, 2018

@GuillaumeGomez all green :)

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Sep 8, 2018

Member

Thanks! :D

Member

GuillaumeGomez commented Sep 8, 2018

Thanks! :D

@GuillaumeGomez GuillaumeGomez merged commit c6c6499 into gtk-rs:master Sep 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment