Skip to content
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

Add bindings for Cairo.Region #621

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

cameronwhite
Copy link
Contributor

This also adds some additional wrapper code for Cairo.RectangleInt in order to implement the unit tests (e.g. constructing a rectangle, comparing equality, and field accessors) - this is the main thing I'd like to discuss since it's a more general topic on the generator side of things!

The extra level of indirection to access the fields (RectangleInt vs RectangleIntData) feels kind of awkward - my current changes have some rough code that attempts to hide this. I feel like it would be more natural if the generator could just directly emit a struct type here, since on the C side this is a plain struct rather than an opaque / handle type

I think it might also make something like cairo_region_create_rectangles() simpler to wrap as well since the memory layout would match
The old cairo bindings from Mono / GtkSharp also made this a struct type, so changing from value to reference semantics would also make it tricky to port existing code correctly

@badcel
Copy link
Member

badcel commented May 8, 2022

Should wait until #622 is fixed to rely on improved struct generation.

@cameronwhite cameronwhite force-pushed the feature/cairo-region branch 4 times, most recently from b772dfb to 3142030 Compare January 15, 2024 04:00
@cameronwhite
Copy link
Contributor Author

I've rebased this on top of the latest changes - the RectangleInt struct is much more usable now without any manual bindings 👍
The two awkward things I encountered are:

  • Constructing it requires new RectangleInt(Internal.RectangleIntManagedHandle.Create()) for now, as was noted in the release notes
  • Equality comparison doesn't work, I think because it includes the Handle member. So code which could have been written as rect_copy.Should().Be(rect_init) requires comparing all the members explicitly. Mostly just an annoyance for writing unit tests :)

This also adds some additional bindings for Cairo.RectangleInt in order to implement the unit tests (e.g. constructing a rectangle, comparing equality, and field accessors)
@badcel
Copy link
Member

badcel commented Jan 15, 2024

Ah good point! I will add some equality check to the generator which just compares the pointers.

edit: Or perhaps we do the member comparison? I think it could even be relatively easy. It would be nice to read your opinion in #1000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants