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

I've implemented a screenshot function of NSView, UIView and GLKView. #230

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

suzumura-ss
Copy link
Contributor

Hi there.

I've implemented a screenshot function of NSView, UIView and GLKView.
NSOpenGLView does not support.

  • In UIView#-(UIImage*)captureImage, make the image using the GLKView#-(UIImage*)snapshot method in the case of GLKView.
  • I added a -(NSString*)captureBase64PngImage method to NSView and UIView. To FrankHelper, this sends a PNG image with the Base64 encoding.
  • FrankHelper#capture(selector) returns to the test code with Base64 decode this data.

@moredip
Copy link
Owner

moredip commented Jul 1, 2013

@MichaelBuckley can I get your opinion on this pull request since it's in your OS X world?

@ondrejhanslik
Copy link
Contributor

@moredip This is not really OS X related - GLKView is an iOS class. I am pretty sure the code is working although some of the UIView / NSView category methods should get FEX_ prefix.

(I don't understand the need for sending base-64-encoded images back to ruby).

@MichaelBuckley
Copy link
Contributor

@moredip I left a comment, but it otherwise looks good.

@MichaelBuckley
Copy link
Contributor

Oh yeah, I agree with @ondrejhanslik. The category methods should be prefixed.

@moredip
Copy link
Owner

moredip commented Jul 1, 2013

Yes, I'm a bit confused as to what the need is for the base64-encoded image. @suzumura-ss, can you explain?

I think it'd make more sense to separate out the GLKView support from the base64 stuff.

@suzumura-ss
Copy link
Contributor Author

The reason I was Base64-encoded, I did not know how to generate a JSON object with PNG-binary in
MapOperationCommand#-(NSString*)handleCommandWithRequestBody: .

If ViewJSONSerializer#-(NSObject*)jsonify: can handle NSData, captureBase64PngImage can return PNG-binary directly.

GLKView support is extensible to UIView#-(UIImage*)captureImage.
So, for example, snapshot-all-view can capture GLKView.

And, I agree that to put a prefix.
Is it a FEX_ ?

@ondrejhanslik
Copy link
Contributor

Also, instead of checking the availability of method snapshot on every UIView, this could be implemented as a GLKView+FrankImageCapture category.

 @implementation GKLView (ImageCapture)

 - (UIImage *)captureImage {
     return [self snapshot];
 }

I still don't like the Base-64 encoding. Maybe we could use the implementation that is already present in the ImageCaptureRoute?

"screenshot/snapshot-all-views" => will capture all the views
"screenshot/view-snapshot/UID" => will return the previously captured image for the view

def get_view_screenshot(view_selector)
    frank_server.send_get("screenshot/snapshot-all-views")
    view_uids = frankly_map(view_selector, "FEX_UID")

    screenshots = Array.new

    view_uids.each do |uid|
        path = "screenshot/view-snapshot/#{uid}"
        image_data = frank_server.send_get( path )

        screenshots.push(image_data)
    end

    screenshots
end

This supposes adding a method -[UIView FEX_UID] (the UID is already used by both dump and image-capture commands).

What do you think?

@suzumura-ss
Copy link
Contributor Author

Why did not the category of GLKView call GLKView#snapshot, because I thought that to increase the framework depends is not good. However, this might be trivial.
Therefore, I agree to change the category of GLKView.

I also thought the idea to get UID.
However, snapshotAllViews method saves to temp all views.
By taking a snapshot several times, garbage is accumulated on the disk.
As an alternative, to provide a way to remove this garbage, is a good way, I think.

@moredip
Copy link
Owner

moredip commented Jul 3, 2013

IIRC the snapshotAllViews method manages its own garbage - it clears out any old snapshots before it saves the new ones.

Cheers,

Pete

Typed on a little bitty keyboard

On Jul 3, 2013, at 5:00 AM, Toshiyuki Suzumura notifications@github.com wrote:

Why did not the category of GLKView call GLKView#snapshot, because I thought that to increase the framework depends is not good. However, this might be trivial.
Therefore, I agree to change the category of GLKView.

I also thought the idea to get UID.
However, snapshotAllViews method saves to temp all views.
By taking a snapshot several times, garbage is accumulated on the disk.
As an alternative, to provide a way to remove this garbage, is a good way, I think.


Reply to this email directly or view it on GitHub.

@suzumura-ss
Copy link
Contributor Author

I made sure the matter of garbage. It was cleared.
I agree to use ImageCaptureRoute to capture views.

I tried to implement capture method with ImageCaptureRoute and FEX_UID.

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.

None yet

4 participants