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

Introduces CGWindowList API for window capturing and improves `CGImage` and `CGContext` #80

Merged
merged 4 commits into from Apr 24, 2017

Conversation

@application-developer-DA
Copy link
Contributor

application-developer-DA commented Apr 4, 2017

Initially I just wanted to rewrite #52 as it seems that the author is not interested in changing his pull request. So I just implemented CGImage bindings, but today another pull request has been merged (#79) which does exactly the same.

This pull request still improves some things in regards to CGImage, the most important ones are:

  • Now CGImage uses CGImageRelease instead of CFRelease (it's a much safer to use CGImageRelease here, as it does not crash if you pass a NULL image reference to it).
  • Now you can retrieve bytes from CGImage (uses data provider and copies the bytes from it).

Also several improvements in CGContext:

  • Added a function create_image(), which allows you to create an image out of existing CGContext.
  • Now it is possible to specify a data pointer passed as a first parameter to the CGBitmapContextCreate() upon CGContext creation -- very important thing when you deal with images as it allows you to specify your own buffer instead of allocating and copying an additional one from the context (tested it on my machine, it gave me 18.46% of performance boost).

And of course this pull requests adds CGWindowListCreateImage() and related things, so now window capturing is possible.


This change is Reviewable

@application-developer-DA
Copy link
Contributor Author

application-developer-DA commented Apr 18, 2017

Copy link
Member

jdm left a comment

Sorry for the delay in reviewing these changes. Could you squash the last commit into the appropriate earlier commits?

let mut rect = CGRect::new(&CGPoint::new(0., 0.), &CGSize::new(0., 0.));
let result = unsafe {
ffi::CGRectMakeWithDictionaryRepresentation(dict.as_concrete_TypeRef(),
&mut rect as *mut CGRect)

This comment has been minimized.

@jdm

jdm Apr 22, 2017

Member

as *mut CGRect shouldn't be necessary.

@application-developer-DA
Copy link
Contributor Author

application-developer-DA commented Apr 24, 2017

Ok, done.

I rebased and squashed the commits (if I got you right, you wanted me to rebase the commits and then to do a force push) and also I took your remark and removed an unnecessary cast in geometry.rs.

@jdm
Copy link
Member

jdm commented Apr 24, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Apr 24, 2017

📌 Commit 81ded0d has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Apr 24, 2017

Testing commit 81ded0d with merge aa04717...

bors-servo added a commit that referenced this pull request Apr 24, 2017
Introduces CGWindowList API for window capturing and improves `CGImage` and `CGContext`

Initially I just wanted to rewrite #52 as it seems that the author is not interested in changing his pull request. So I just implemented `CGImage` bindings, but today another pull request has been merged (#79) which does exactly the same.

This pull request still improves some things in regards to `CGImage`, the most important ones are:
* Now `CGImage` uses `CGImageRelease` instead of `CFRelease` (it's a much safer to use `CGImageRelease` here, as it does not crash if you pass a `NULL` image reference to it).
* Now you can retrieve bytes from `CGImage` (uses data provider and copies the bytes from it).

Also several improvements in `CGContext`:
* Added a function `create_image()`, which allows you to create an image out of existing `CGContext`.
* Now it is possible to specify a data pointer passed as a first parameter to the `CGBitmapContextCreate()` upon `CGContext` creation -- very important thing when you deal with images as it allows you to specify your own buffer instead of allocating and copying an additional one from the context (tested it on my machine, it gave me 18.46% of performance boost).

And of course this pull requests adds `CGWindowListCreateImage()` and related things, so now window capturing is possible.

<!-- 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-graphics-rs/80)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 24, 2017

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

@bors-servo bors-servo merged commit 81ded0d into servo:master Apr 24, 2017
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.