cparnot / ASCIImage

Create UIImage / NSImage instances with NSString and ASCII art
MIT License
1.52k stars 76 forks source link

Adds a scale parameter to the image #1

Open mz2 opened 9 years ago

mz2 commented 9 years ago
cparnot commented 9 years ago

iOS too?

mz2 commented 9 years ago

Damn it, no. One moment...

mz2 commented 9 years ago

Oh, wait, but actually the iOS implementation has already scale: as part of the method selector. I passed it as a value you can include with the context handler instead of a separate argument (as I treated it similarly as an optional value -- there's an implicit scale of 1.0).

mz2 commented 9 years ago

So technically the iOS and Mac implementations are inconsistent (on iOS there's a scale: parameter, on OSX not, and on OSX the scale is in this branch accepted as an optional context value where previously no scaling existed on OSX). I would propose an API change where scale is made indeed a value passed into the context handler instead of it being a named parameter to the method.

cparnot commented 9 years ago

So technically the iOS and Mac implementations are inconsistent

It's more subtle that this. On iOS, the scaling has to be done by your code based on the device scale. On OS X, the NSImage implementation takes care of it and you render in the block at scale 1 no matter what (NSImage takes care of scaling based on the screen). This scaling is only exposed for the purpose of tests. On OS X, the tests render the NSImage instance in various... NSImage with different sizes, to mimick different screens.

The "scale" you want to apply is different. It's not the screen scale, it's a custom scaling factor on top of that. Both implementations are consistent: no such factor exists at the moment. But it could be added. Also on iOS where it is not yet present in your branch ;-)

mz2 commented 9 years ago

Ok thanks for clarifying -- then I should indeed add this scale factor to iOS too. Is there an iOS target someplace public already, or shall I make one?

cparnot commented 9 years ago

Is there an iOS target someplace public already, or shall I make one?

Yes. And there are tests :-)

mz2 commented 9 years ago

zomg!