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

Additional functions for `CGImage` and `CGDataProvider` #219

Merged
merged 3 commits into from May 29, 2018

Conversation

@application-developer-DA
Copy link
Contributor

application-developer-DA commented May 28, 2018

  1. Adds a possibility to construct a CGImage from a given provider. I don't know if it's the most optimal way to implement it and if it is safe, but it would be nice to wrap this function somehow, that was the first attempt to do it.
  2. Adds some rendring intent constants.
  3. Adds a function which allows to create a data provider from a slice. This is highly unsafe, that's why I marked it with unsafe. And I'm not sure if it is the best way to do this. Perhaps the one can store something like Option<slice> in the data provider, but in this case the provider will get some lifetime parameter (and we have to update the foreign type definition). I'm not sure that having slice parameter and a lifetime would make it 100% safe though, because when we're passing a data provider ref to some FFI, they could theoretically retain a data provider (or the image which uses the data provider), expecting that the data inside the data provider lives long enough. Probably the only "really safe" way to manage the data provider data is the one which is currently available (via Arc<Vec<u8>>), but this limitation of having Arc<Vec<u8>> may be very ineffective in some sort of appliations, so for instance in my case I had to add the additional "constructor" to allow creation of unsafe data provider and I guarantee the safety in my code by some other means.

This change is Reviewable

@jdm
Copy link
Member

jdm commented May 29, 2018

@bors-servo r+
Thanks! Your reasoning about the safety of the new API makes sense.

@bors-servo
Copy link
Contributor

bors-servo commented May 29, 2018

📌 Commit ff9ff84 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented May 29, 2018

Testing commit ff9ff84 with merge 410c1e8...

bors-servo added a commit that referenced this pull request May 29, 2018
Additional functions for `CGImage` and `CGDataProvider`

1. Adds a possibility to construct a CGImage from a given provider. I don't know if it's the most optimal way to implement it and if it is safe, but it would be nice to wrap this function somehow, that was the first attempt to do it.
2. Adds some rendring intent constants.
3. Adds a function which allows to create a data provider from a slice. This is highly unsafe, that's why I marked it with unsafe. And I'm not sure if it is the best way to do this. Perhaps the one can store something like `Option<slice>` in the data provider, but in this case the provider will get some lifetime parameter (and we have to update the foreign type definition). I'm not sure that having slice parameter and a lifetime would make it 100% safe though, because when we're passing a data provider ref to some FFI, they could theoretically retain a data provider (or the image which uses the data provider), expecting that the data inside the data provider lives long enough. Probably the only "really safe" way to manage the data provider data is the one which is currently available (via `Arc<Vec<u8>>`), but this limitation of having `Arc<Vec<u8>>` may be very ineffective in some sort of appliations, so for instance in my case I had to add the additional "constructor" to allow creation of unsafe data provider and I guarantee the safety in my code by some other means.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/219)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 29, 2018

☀️ Test successful - status-travis
Approved by: jdm
Pushing 410c1e8 to master...

@bors-servo bors-servo merged commit ff9ff84 into servo:master May 29, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.