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.

Sign up for GitHub

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

Jump to bottom

swap cache position #1

Merged
merged 5 commits into from
Aug 1, 2012
Merged

swap cache position #1

merged 5 commits into from
Aug 1, 2012

Conversation

jcupitt
Copy link
Contributor

@jcupitt jcupitt commented Jul 20, 2012

I think putting the cache earlier should give a speedup. I'd like to benchmark before suggesting you merge, but I can't see how to get carrierwave-vips-benchmark to use my fork of carrierwave-vips, unfortunately.

@jcupitt
speed up for large shrinks
2e05ed7 
put the tile cache just after the big shrink for a nice speedup
@jcupitt
oops typo
ed849a7
@jcupitt
benchmark the other way around
e0487da
@jcupitt
reorder again, should now be best
a51f4a7
@jcupitt
jpeg subsampling was not working
cdf6935 
the thing that selected a subsample factor for jpg images always chose
factor 1

also, add some logging (disabled for now) so we can verify operation
@jcupitt
Copy link
Contributor Author

jcupitt commented Jul 21, 2012

OK, all done now I think. I've updated the carrierwave-vips-benchmark README, it seems to all be working with these changes:

https://github.com/stanislaw/carrierwave-vips-benchmarks

Thank you very much for making this into a proper gem and fixing my ruby.

@eltiare
Copy link
Owner

eltiare commented Jul 23, 2012

I'll review the changes get this into the gem.

@stanislaw
Copy link
Contributor

Any updates on this?

I am going to write Carrierwave guys about carrierwave-vips as a good alternative to default RMagick. What I want, is at least to have an outline about carrierwave-vips in their wiki.

Thanks.

@eltiare eltiare merged commit cdf6935 into eltiare:master Aug 1, 2012
@eltiare
Copy link
Owner

eltiare commented Aug 1, 2012

It's done.

@jcupitt
Copy link
Contributor Author

jcupitt commented Aug 2, 2012

Thank you Jeremy. Sorry the patch was so messy, I didn't realise my later
cmmits would be added to the pull request.

On Thursday, 2 August 2012, Jeremy Nicoll wrote:

It's done.

Reply to this email directly or view it on GitHub:
#1 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jcupitt @eltiare @stanislaw