cacao icon indicating copy to clipboard operation
cacao copied to clipboard

Published 20 hours ago • verified publisher icon ryanmcgrath

Replace `core-foundation` and `core-graphics`

Open madsmtm opened this issue 2 years ago • 6 comments

Message sending now requires types to implement Encode, which these crates don't do (yet, though maybe they will, see https://github.com/servo/core-foundation-rs/pull/628). Since these crates are used quite infrequently, and we'll probably want to use icrate later on anyhow, this PR replaces them.

madsmtm avatar Jul 20 '22 04:07 madsmtm

This one makes sense to me, though I'm wondering if it doesn't make sense to batch all 3 of these PRs into one big PR and just merge that... I could be wrong but I think there's some things repeating across them (e.g *const Class to static changes). No idea if that'll make merging hell or not - though I'll defer to you given I don't want to ask for too much free work here. ;P

ryanmcgrath avatar Aug 21 '22 22:08 ryanmcgrath

Don't worry about things being difficult to merge or not, I'll figure that out once https://github.com/ryanmcgrath/cacao/pull/30 is done (which is probably also why things are confusing rn; because that PR is "further ahead" than these).

madsmtm avatar Aug 22 '22 08:08 madsmtm

Will do! Thanks!

On Mon, Aug 22, 2022 at 01:37, Mads Marquart @.***> wrote:

Don't worry about things being difficult to merge or not, I'll figure that out once #30 is done (which is probably also why things are confusing rn; because that PR is "further ahead" than these).

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

ryanmcgrath avatar Aug 22 '22 08:08 ryanmcgrath

I think this one was what I wanted to do after #30.

Note the change in API for Image::draw, which is unfortunate. It's not a strictly a needed change, but I wanted to avoid the direct dependency on core-graphics since it locks the public API to the specific version cacao depends on, and is used so little.

madsmtm avatar Sep 11 '23 23:09 madsmtm

A few more notes:

  • NSRect and CGRect (and the others) are exactly equivalent, it's just that the version of objc2 that we're using here doesn't have the aliases defined for it.
  • NSSize currently restricts its input to be valid floating points; this turned out to be a bad idea, and I've removed that in a future version of objc2

madsmtm avatar Sep 12 '23 00:09 madsmtm

Note the change in API for Image::draw, which is unfortunate. It's not a strictly a needed change, but I wanted to avoid the direct dependency on core-graphics since it locks the public API to the specific version cacao depends on, and is used so little.

Hmm, you're specifically referring to not needing CGContextRef? I see what you mean, though it is definitely unfortunate... but then again, if there were to be other spots that use CGContextRef, it might make sense to bite the bullet and at the very least feature-flag it.

How often does core-graphics change anyway? It might not be a big deal to have it as a dependency.

ryanmcgrath avatar Sep 12 '23 20:09 ryanmcgrath