cairo icon indicating copy to clipboard operation
cairo copied to clipboard

Use destructors for automaticaly closing of resources

Open planetis-m opened this issue 5 years ago • 8 comments

in order to do these are the steps that need to be taken:

  1. types and procs interfacing directly with libcairo need to keep their original names example type example proc

  2. The "higher level" user api of this library will get shorter names like Surface, imageSurfaceCreate.

  3. destructors will be attached to these types. it will look like this:

type
  Surface = object
    data: PSurface

`=destroy`(surface: var Surface)
`=`(dst: var Surface, src: Surface) {.error.}

These changes will break the api of this library but will allow the user to forget the closing of resources.

planetis-m avatar Jan 02 '20 21:01 planetis-m

I like it but maybe it should be under nim-lang/cairo2?

Araq avatar Jan 02 '20 21:01 Araq

This would not be a big win for me. Usually one creates 1 surface for the entire app. Its not hard to destroy it at the end of your program ... or event just forget and quit and have the OS do it for you.

I never had a problem with leaking Surfaces

treeform avatar Jan 03 '20 16:01 treeform

@treeform But are surfaces the only exposed resource? The surface is only an example here.

Araq avatar Jan 03 '20 17:01 Araq

@araq Here are all of the destroys in the library:

proc destroy*(cr: ptr Context)
proc destroy*(options: ptr FontOptions)
proc destroy*(font_face: ptr FontFace)
proc destroy*(scaled_font: ptr ScaledFont)
proc destroy*(path: ptr Path)
proc destroy*(surface: ptr Surface)
proc destroy*(pattern: ptr Pattern)

I never had an issue with any of them, so I feel like it will not impact me in any way. It might be a cool thing to do, even if it's not a useful thing to do.

Will the =destroy only work with --gc:arc?

treeform avatar Jan 03 '20 18:01 treeform

I made a minimal example. Maybe this is more correct, because cairo_reference increments the ref count and cairo_destroy dec its, and iff it's equal to zero then deallocs the memory?

type
  Context* = object
    impl: PContext

proc `=`(cr: var Context, original: Context) =
  if cr.impl != nil: cairo_destroy(cr.impl)
  cr.impl = cairo_reference(original.impl)
proc `=destroy`(cr: var Context) =
  if cr.impl != nil:
    cairo_destroy(cr.impl)
    cr.impl = nil
proc `=sink`(cr: var Context; original: Context) =
  `=destroy`(cr)
  cr.impl = original.impl

planetis-m avatar Jan 03 '20 20:01 planetis-m

Will the =destroy only work with --gc:arc?

No, it's always active, however the old seq implementation (and stuff building on top like tables) ignores destructors.

Araq avatar Jan 04 '20 10:01 Araq

Fork exists at https://github.com/b3liever/cairo2 @treeform try it if you like

planetis-m avatar Jan 08 '20 18:01 planetis-m

Don't forget that we have some sort of high level cairo in gintro for some years now. I have not spent too much effort in it yet, as it does not support introspection and toggle_references, so it is different from the other gtk stuff. But it seems to work with ARC now. I don't accept pull requests, but I may accepts issues or suggestions like the "=" proc to create errors. May add it soon, also for GTK widgets and that.

StefanSalewski avatar Jan 28 '20 11:01 StefanSalewski