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

A set of fixes to keep resized image state properly #163

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

Conversation

zaak
Copy link

@zaak zaak commented Jan 2, 2015

There was a few issues related to keeping image state after any of the resizing tranformations (like resize, crop, rotate). The revert method worked improperly in this case, applying only one transformation on original pixel data. This logic failed if image was modified more that once (for example crop, then rotate, then resize etc).
What I did was just simply adding an additional canvas especially for rendering. Resized image state is kept by an internal canvas (with all modifications like crop, resize, rotate, but without any filters applied), so there are always "fresh" pixels ready for use in revert. The second canvas (visible one) is used for rendering - it takes image from first canvas and applies filters/presets to create the final image.

Also merged plugins submodule, updated conflicting node packages and fixed build scripts for travis.

Small demo: http://zaak.github.io/CamanJS-demo/

@EmanH
Copy link

EmanH commented Apr 16, 2017

You're a legend mate! Thank you so much.

@av01d
Copy link

av01d commented Dec 12, 2018

It seems that the revert function is broken after this patch? I'm using caman.full.js from this patch.
Calling revert results in a blank canvas.
If I fetch caman.full.min.js from the demo on http://zaak.github.io/CamanJS-demo/ (http://zaak.github.io/CamanJS-demo/caman.full.min.js), revert works. Unfortunately, only the minified version seems to be available here.
If I use https://raw.githubusercontent.com/meltingice/CamanJS/1ac3fb607da642597b02e6e0e4f875e111b27324/dist/caman.full.js or https://raw.githubusercontent.com/meltingice/CamanJS/1ac3fb607da642597b02e6e0e4f875e111b27324/dist/caman.full.min.js, revert is broken as described above.
Would @zaak be so kind to share his latest caman.full.js (unminified) with me (and others)?
Thank you so much!!!

@av01d
Copy link

av01d commented Dec 17, 2018

@zaak It would be such a great christmas present... sharing caman.full.js with the fixes to keep resized image state properly with all of us. Please?
Thanks Santa!

@zaak
Copy link
Author

zaak commented Dec 17, 2018

Hey @av01d. Sorry, I didn't notice your previous message.
Here's a fork with all the fixes that we use for CKFinder purposes, have a look here. 🎅

@av01d
Copy link

av01d commented Dec 18, 2018

Thanks @zaak for this nice christmas gift ;-)
Unfortunately, your caman.full.js (https://raw.githubusercontent.com/ckfinder/CamanJS/master/dist/caman.full.js) still has the same bug: revert does not work, it causes an empty canvas.
I'll try to create something myself.

@av01d
Copy link

av01d commented Dec 19, 2018

As for the non-working revert: I finally figured it out, I was doing something wrong. I applied a filter like so:

Caman('#filterImg', function() {
   this.sepia(40);
   this.render();
});

And when I wanted to revert that filter:

Caman('#filterImg', function() {
   this.revert();
});

This last call calls the Caman constructor again, re-initializing the image in #filterImg.
The weird thing is that this did work with the original caman.full.js.

The correct approach (for both the original caman.full.js and the version made by @zaak) is this:

var caman = Caman('#filterImg', function() {
   caman.greyscale();
   caman.render();
});

And if you want to revert a filter, do this:

caman.revert();

You now have only 1 Caman instance, its constructor is called only once and everything works!

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.

4 participants