Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

[Sad Trombone] Image pixel data access #57

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

Conversation

saniul
Copy link
Contributor

@saniul saniul commented Mar 15, 2015

  • Swift API (Feedback wanted)
  • JS Bridge
  • Documentation

It’s a bit awkward that Pixels store color info in Int8 [0, 255] instead of Double [0.0, 1.0], but I provided a .color getter and setter, which could be enough... Thoughts?

PixelBitmap has a transform method, which constructs a new PixelBitmap by applying the transformation on the original. The additional convenience overloads of transform provide information about the index (0..<width*height), or the row/column of the pixel. What do you think? Should this be simpler?

@saniul
Copy link
Contributor Author

saniul commented Mar 15, 2015

Ref: #48

let bitmapInfo = CGBitmapInfo(CGImageAlphaInfo.PremultipliedLast.rawValue)
let bytesPerRow = width * 4

let context = CGBitmapContextCreate(data, width, height, 8, bytesPerRow, colorSpace, bitmapInfo)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This context is leaking :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right you are, @NachoSoto, thanks!
’tis not the context that is leaking (CF objects are automatically memory-managed), but the data allocated on #57 (diff).

I added a PixelDataDestroyer class to properly clean up the underlying PixelBitmap data on deinit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right, I was thinking of UIGraphicsEndImageContext()


init(image: Image) {
width = Int(image.size.width)
height = Int(image.size.height)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful: Image.size reports sizes in points, but for these CG APIs, you'll need to work in pixels.

@saniul
Copy link
Contributor Author

saniul commented Mar 25, 2015

Ugh, on-device performance is horrendous :(
Inverting every pixel in a 400x400 image using

function(pixel) { 
    pixel.red = 1.0 - pixel.red
    pixel.green = 1.0 - pixel.green
    pixel.blue = 1.0 - pixel.blue
    return pixel 
} 

takes ~14 seconds on an iPad Air.

Even just mapping over all pixels without modifying them takes 6-7 seconds :| Most time is spent in -[JSValue callWithArguments:]

@saniul
Copy link
Contributor Author

saniul commented Mar 25, 2015

Added a sample scene to Examples/

@andymatuschak
Copy link
Contributor

I'm not super surprised, but that's a bummer. Two ideas, depending on exactly what's slow here:

  1. In the likely event that it's bridging millions of different Pixel objects that's slow, you could try making these functions consume function(x, y, r, g, b) instead.
  2. In the also-likely event that millions of context switches is also intractable, you could instead expose an API to access a JavaScript array of rgb data (a bit like Processing's pixels[])

@saniul
Copy link
Contributor Author

saniul commented Mar 25, 2015

Yup. I think 2. is going to be the right decision in the end, but I'll try 1. first.

@sophiebits
Copy link
Contributor

(Btw, if you're going with 2, on React Native we saw that serializing to JSON and transferring only one string over the bridge was faster than building up the arrays/objects manually using the JSC API.)

@saniul
Copy link
Contributor Author

saniul commented Mar 25, 2015

@spicyj thanks for the tip!

@andymatuschak
Copy link
Contributor

Interestingly, there's actually a standard ImageData API. That's what <canvas> elements can give you; it's also what Paper.js exposes.

@kevinbarabash
Copy link
Contributor

CIFilter might come in handy. It covers quite a few image processing tasks. Here's a link to the list of filters:
https://developer.apple.com/library/ios/documentation/GraphicsImaging/Reference/CoreImageFilterReference/index.html#//apple_ref/doc/uid/TP40004346

@andymatuschak
Copy link
Contributor

Yep, good point. Can only filter things that are already raster of course, and CI isn't suitable for realtime applications, but there are ways to mitigate both those issues if we find we need it.

On Mar 25, 2015, at 10:54 PM, Kevin Barabash [email protected] wrote:

CIFilter might come in cover quite a few image processing tasks. Here's a link to the list of filters:
https://developer.apple.com/library/ios/documentation/GraphicsImaging/Reference/CoreImageFilterReference/index.html#//apple_ref/doc/uid/TP40004346


Reply to this email directly or view it on GitHub.

@kevinbarabash
Copy link
Contributor

@andymatuschak I didn't realize that. I did a quick search and Apple has some suggestions on how to make it work for realtime applications. They suggest using an EAGL context so all the bits stay on the GPU. The details are outlined in "Creating a Core Image Context on iOS When You Need Real-Time Performance" on https://developer.apple.com/library/ios/documentation/GraphicsImaging/Conceptual/CoreImaging/ci_tasks/ci_tasks.html.

@kevinbarabash
Copy link
Contributor

There are probably also some situations where you don't necessarily need to apply real-time effects in which case plain old CI would work just fine.

@andymatuschak
Copy link
Contributor

Right, thanks! I was responsible for UIKit's rendering concerns at Apple, so I'm familiar, just not looking forward to digging that ditch again. Ah well.

Seems like there’s three use cases here:

  1. Sampling a specific pixel of an image. This API is good enough for that.
  2. Reading and performing some kind of computation on a whole image (e.g. what's the nearest opaque pixel to the user's touch?) The two solutions @saniul's exploring should make that case workable.
  3. Filtering an image. We've also been talking about wanting custom blend modes for our layers. I wonder if we should consider these cases together, at least as far as the API. I guess CA exposes this as filters vs. compositingFilters. We could implement the former using CI (though it'd mean spinning up a GL context for each layer requiring realtime filtering) and the latter using CA SPI (for now anyway). If this becomes an important use case, we can switch over to a more robust rendering backend (e.g. SpriteKit or Cocos2D) that would make it easier to do this kind of filtering regularly. Maybe let's tackle that in a different PR?

@saniul
Copy link
Contributor Author

saniul commented Mar 27, 2015

Maybe let's tackle that in a different PR?

👍

@saniul
Copy link
Contributor Author

saniul commented Apr 2, 2015

Sorry for the wait everyone, life/work just keeps getting in the way 😞

Went with this suggestion:

In the also-likely event that millions of context switches is also intractable, you could instead expose an API to access a JavaScript array of rgb data (a bit like Processing's pixels[])

Still very slow on the device :( Inverting a 400x400 image of Pusheen takes 11+ seconds on iPad Air:
7s to load the pixel array (most time spent creating the JSValue array from Cocoa/Swift array)
0.3s to run over the pixels and modify them in js
Most of the remaining 4s is spent bridging the resulting value back to Cocoa/Swift-land

@spicyj I tried constructing a JSON string representation of that but it just kept crashing. I’m not suprised, though since it’s pretty ridiculous (an array of 160000 arrays of 3 ints).

I will still try 1. but I don’t expect any significant improvements. At this point I’m not sure if there’s any point in keeping this API if it runs so slow on-device. Maybe if it only lived in the Swift layer...? 👎

@sophiebits
Copy link
Contributor

@saniul When JSONifying, you could try sending each pixel down as a 32-bit (24-bit?) int and then convert it using pure JS to the nested arrays? Maybe that would be lower overhead. Too bad there's no JSC API for typed arrays.

@sophiebits
Copy link
Contributor

(0.3s to run over the pixels is small relative to 11s but still pretty large relative to 16ms…)

@andymatuschak
Copy link
Contributor

Rough. Thanks a lot for trying this out, @saniul. No need to apologize for delays! :)

The performance characteristics of #1 should be pretty different: the only cost should be context switching, as opposed to all the copies here. It'll obviously be much worse than the 0.3s time here, but maybe it'll be OK?

I can imagine more aggressive ways for us to attack this (e.g. implementing a new JS "data" type which knows how to be backed by some buffer), but if approach #1 doesn't work we should probably surrender for now. :/

@saniul
Copy link
Contributor Author

saniul commented Apr 3, 2015

@spicyj sending ints over the “wire” and then breaking them down in the JS later didn’t really help.

@andymatuschak I implemented colorAt(pos) and setColorAt(pos, r, g, b). Inverting 400x400 pixels takes ~6.3s on the same device.

@andymatuschak
Copy link
Contributor

Hm. One more idea: you'd have half as many context switches and would move the work of iteration to Swift if you made transform take a function(x, y, r, g, b) (deliberately x,y here instead of a Point to avoid tons of allocations)

@andymatuschak
Copy link
Contributor

(also: thanks for trying all this stuff @saniul!!!)

@saniul
Copy link
Contributor Author

saniul commented Apr 3, 2015

oh @andymatuschak pos is actually an Int (treating the data as an array). there are overloads with an additional parameter for row, column getting/setting

@andymatuschak
Copy link
Contributor

Ah, gotcha.

On Apr 3, 2015, at 12:02 PM, Saniul Ahmed [email protected] wrote:

oh @andymatuschak https://github.com/andymatuschak pos is actually an Int (treating the data as an array). there are overloads with an additional parameter for row, column getting/setting


Reply to this email directly or view it on GitHub #57 (comment).

@saniul
Copy link
Contributor Author

saniul commented Apr 3, 2015

Also, no problem! I’m very happy to help.

@saniul saniul changed the title [WIP] Image pixel data access [Sad Trombone] Image pixel data access Apr 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants