That sounds awesome what you want to do! Thank you!!
The best way to contribute code is to send me the patches, via email to
the mailing list (pain...@googlegroups.com) or with bug reports to which
you attach the patches.
I did not start work on layers (yet), and I don't know if someone else
did. I know that Gordon Clark (aka Munksey) is working on selection
rotation. He can send us the patches when he is ready. He already did
selection invert.
Please let me know when you have the patches ready. I look forward to see
them. :)
Thanks again guys!
Best regards,
Mihai
Le Fri, 24 Sep 2010 05:25:35 +0300, <ar...@gibbutz.com> a écrit:
> Hey there,
>
> I'm currently integrating your awesome drawing library into our (not yet
> launched) quiz building application. I'm planning on adding "layers",
> rotation and subject specific toolsets (for circuit diagrams, free body
> diagrams, molecular models etc). I would be happy to contribute back to
> this project, so let me know how to go about doing that. Also if you
> had any suggestions (i see "layers" on the TODO list, so I was wondering
> whether someone's already nearly finished with that yet).
>
> Arjun
>
--
Mihai Sucan
http://www.robodesign.ro
Le Fri, 24 Sep 2010 16:07:16 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey All,
>
> I am planning to work on the rotation this weekend. I'll let y'all know
> if I
> have any problems ;)
>
> Arjun, not sure if you've looked at google docs Drawing documents, but
> they have
> set up a general drawing app which has a "shapes" panel. Perhaps we
> could create
> a shapes tab which allows for the kind of diagram models you are talking
> about,
> but it would integrate in a way that makes sense to the application.
Yeah, that sounds like a very good idea. Google Docs uses vectors. We want
to keep, at the moment, bitmapped-based drawing. Going into SVG land would
be awesome, but that introduces lots of unneeded complexity. While some
favor vectorial graphics, I believe there's room for bitmap-based image
editors as well.
> In my opinion, this tool could serve for many different purposes if we
> integrate
> the tools in a very general manner. Although, perhaps allowing the
> flexibility
> for the person setting up the application to arrange the tools in a
> manner that
> is best for them to represent it would be an awesome idea, as well. I
> imagine
> that would be quite an overhaul, and might be unnecessary.
True. I thought of making the UI ultra-customizable, but I think we should
not try to please everyone. We need to make the default user experience
and UI as best as possible. We need the best performance we can get out of
the code, to make the paint tool feel "light", to keep it "easy to use".
> Just so you guys know, here are some ideas I had planned on implementing:
> - have an option for talk bubbles for the text tool
> - image rotations
> - function callbacks for the "insert image" button (since ajax can be
> funky with
> proxies, allowing the user to send the request to a local script might be
> better)
> - an effects tab for the selection tool (make negative, increase
> brightness,
> etc)
That's some cool stuff! These things fit exactly right up the alley for
PaintWeb. I believe the code is pretty much ready for such features.
If any of you has questions, or would like to discuss ideas, please let me
know. I'm available on Gtalk, Yahoo, IRC and Skype. I want to help you as
much as possible.
What I am thinking is that if both of you want to work on the code, we
should find a way to coordinate and share the code changes. We can use
git, mercurial or svn, whatever you guys prefer. We can also coordinate
using bug reports, patches, and code reviewing tools. Github or Google
Code, just name it, and I'll try to help as much as possible in setting-up
anything you guys would prefer.
Best regards,
Mihai
I think I'll look into moving the code from SVN to Mercurial - as allowed
by Google Code. Additionally, I'll look into mirroring the Mercurial
repository on Github, for those who prefer Git.
Thanks Gordon for your willingness! That's really appreciated.
Best regards,
Mihai
Le Sat, 25 Sep 2010 00:55:00 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> I'm not very familiar with google code or how you go about
> submitting/merging
> changes. I do use SVN a lot and know that github has very similar
> commands. I
> personally think that github would be a great way for people to follow
> the
> project and for us to be able to merge changes.
>
> Ultimately though, I'm indifferent, I'm willing to learn whichever
> version
> control utility you guys think would be most effective.
--
Mihai Sucan
http://www.robodesign.ro
Le Mon, 27 Sep 2010 03:16:31 +0300, Arjun Vasan <ar...@gibbutz.com> a
écrit:
> Hey guys,
>
> Nice to see all the enthusiasm! I haven't yet fully delved into the
> paintweb code, but I think Gordon's shape tab idea seems good for the
> majority of applications. We could set it up so that users could add a
> particular shape set based on their needs (for example, a circuit
> component
> set). They could create/modify their own shape sets by leveraging the
> existing "select" feature (click button in the "selection" panel to add
> to a
> shape set).
Indeed, good idea.
> With regards to overall strategy, I think keeping it light is a really
> good
> idea. As it is, I was able to implement paintweb into my app (with some
> tiny mods) within an hour, which was awesome.
Great to hear that!
> Currently I'm planning on integrating 3 libraries into paintweb in the
> next
> couple weeks:
> 1. MathQuill <http://laughinghan.github.com/mathquill/> - An inline
> equation editor (useful for math/science diagrams)
> 2. ChemDoodle <http://web.chemdoodle.com/> (Web Components)- A canvas
> based
> tool to draw all sorts of molecular models
> 3. Flot <http://code.google.com/p/flot/> - A plotting library for both
> math
> functions & datasets
This list looks delicious! Send your patches and I'll look into reviewing
and integrating them into the main code, if applicable.
Please let me know how things are going, and how I can help.
> I would say more, but to avoid looking like an idiot I'm going to first
> play
> around with the existing code for a couple days. As for the repository,
> whatever is decided is fine with me.
Great! I'll setup a Github repository for easy cloning and to allow easily
for contributors to start hacking on PaintWeb. Perhaps I'll also move
PaintWeb SVN to Mercurial on Google Code.
This week I'm still very busy with work. During the weekend I really want
to get the chance to do these things.
Best regards,
Mihai
This sounds like a great improvement over your previous patch. Thanks for
the patches!
I'll look into the code over the weekend - this week I am still very busy.
I'll provide you with a review/feedback and hopefully ideas on how we can
improve the code further.
Best regards,
Mihai
Le Tue, 28 Sep 2010 01:44:49 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Guys,
>
> I actually decided to write my own getImageData() and putImageData()
> functions,
> which account for rotation. So this new patch actually works differently
> from
> the last one I sent. It actually rotates the marquee and accounts for
> pixel
> manipulation. It only works for clearing the area, filling the area and
> the
> inversions. Although, when performing the inversions, you'll see (much
> like
> before), that the interpolation from rotation causes quite a bit of
> quality loss
> at certain angles.
>
> I'm going to continue to work on this so it works for copying, cutting
> and
> pasting, but in case you guys had any thoughts on reducing image loss, I
> thought
> I'd send this along... Anyhow, I'll stop sending e-mails til it's done :)
--
Mihai Sucan
http://www.robodesign.ro
Today I got the chance to look at your patches. Here are my comments for
your second patch you sent on the 27th of September:
- the same ideas as in the previous review apply.
- the new selectionRotate() method looks good, but it doesn't need to
create its own canvas and 2d context. I would suggest clearing the
bufferContext, then apply the context transformations (rotate() and
translate()), and finally redraw the selection buffer (sel.context). The
bufferContext is used for the purposes of drawing.
- similarly to the last patch you've provided, you should check if the
user is manipulating pixels or the selection. Thus, like in the third
patch you've provided, you rotate only the marquee if the user manipulates
the selection.
The idea here is that the main selection code redraws onto the buffer the
selected pixels. Given that the rotate() transformation is applied for the
buffer context, the browser will always nicely rotate the selected pixels.
You won't need to do a lot of other changes, just set rotate() and
translate() for the bufferContext, and needsRedraw = true.
This should hopefully solve problems #1 and #2 you pointed out
Other than that, the patch looks really good. Thanks a lot! Please let me
know how this goes.
Best regards,
Mihai
Le Mon, 27 Sep 2010 04:35:13 +0300, Gordon Clark <ucl...@yahoo.com> a
This is the review for the patch you sent on the 28th of September:
(I'll just comment on the code changes you did from the previous patch you
sent)
- I like what you do with the marquee. Rotation works fine, yay!
- the rest of the changes - for using the rotationContext - do not seem
needed. If you look into my previous review, you'll see that I propose a
different approach.
+ if (marqueeStyle.transform != undefined) {
+ marqueeStyle.transform = style;
I think it's better to check like this:
+ if ('transform' in marqueeStyle) {
That's all. The changes you did in this patch are definitely for the
better - especially with regards to the marquee.
Thanks a lot!
Best regards,
Mihai
Le Tue, 28 Sep 2010 01:44:49 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Guys,
>
> I actually decided to write my own getImageData() and putImageData()
> functions,
> which account for rotation. So this new patch actually works differently
> from
> the last one I sent. It actually rotates the marquee and accounts for
> pixel
> manipulation. It only works for clearing the area, filling the area and
> the
> inversions. Although, when performing the inversions, you'll see (much
> like
> before), that the interpolation from rotation causes quite a bit of
> quality loss
> at certain angles.
>
> I'm going to continue to work on this so it works for copying, cutting
> and
> pasting, but in case you guys had any thoughts on reducing image loss, I
> thought
> I'd send this along... Anyhow, I'll stop sending e-mails til it's done :)
--
Mihai Sucan
http://www.robodesign.ro
This is the review for the last patch you sent (30th of September):
- I appreciate you've made changes to config-example.json. Please continue
to do so.
- "Rotate N degrees on paste" is not needed, I believe. Once you address
the comments in the previous reviews, anything the user pastes will be
automagically rotated.
- I believe you do not need to apply transform on layerContext, but on
bufferContext.
+ returnValue.width = (widthOne > widthTwo) ? widthOne : widthTwo;
I think you can use returnValue.width = Math.max(widthOne, widthTwo). Same
goes for returnValue.height.
- I like the changes you did to the selection invert methods. They now
take into account the "image manipulation" checkbox. Yay! Good work!
+ if (!sel.layerCleared) {
+ app.historyAdd();
+ } else {
+ sel.context.drawImage(contextToManipulate, sel.x, sel.y,
+ sel.width, sel.height, 0, 0, widthOriginal, heightOriginal);
+ }
widthOriginal and heightOriginal are both undefined, and that line throws
an exception when the user tries to flip the selection. I presume this is
just something you forgot to update in the code?
Other than this, the patch looks much better than the first one. ;) Thanks
a lot!
Now ... you may wonder what you have to do: please read my reviews and try
to apply them - send me emails with questions if you want, or ask me on
gtalk/yahoo. Take your latest code and apply the changes in a single
branch.
Ideally, clone the PaintWeb mercurial/git repo (whatever you prefer) ...
and make your changes on the clone. Send me an email when you have
something ready for me to see, with a link to the clone branch you work on
- no need for us to exchange patches as attachments via email anymore. :)
That way I can directly comment on the diffs - both google code and github
allow us to collaborate much better, to do code reviews within the diffs
(online!). They also allow me, as an integrator, to much more quickly
retrieve the changes you do.
I hope these reviews don't come as "hard" for you. It is not my intent! :)
I hope that my emails give you ideas on how to take the patch forward.
Thanks a LOT! You are quite close to getting it done. :)
Best regards,
Mihai
Le Thu, 30 Sep 2010 17:32:57 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Guys,
>
> I decided to backtrack on the rotation again. I was working with the
> marquee and
> found that it really wasn't intuitive from a user perspective to be able
> to spin
> the selection tool itself. Once you are upside down you are working in a
> pretty
> weird environment.
>
> So, I made it so that you can rotate a certain degree when you paste
> content and
> that it's a configuration next to transparent/image manipulation. When
> you paste
> the content, it resizes the selection box to fit the rectangular
> rotation. You
> still get some level of image loss, but I don't think this is avoidable
> as you
> need to compensate for pixels that get split on certain angles.
>
> I also tweaked the inversions so that it works with "Image
> Manipulation," which
> I had not taken into account before.
>
> Anyhow, when you are less busy with work, let me know your thoughts :)
> I'll see
> what else I can change.
>
> - Gordon
Le Mon, 04 Oct 2010 03:43:39 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> I've forked the project on git: http://github.com/Munksey/PaintWeb
>
> I just pushed some of the changes I had made, while making some of the
> fixes
> you've suggested:
> - I added imageResizeable and viewportResizeable to the
> config-example.json file
> - Got rid of config.json.
> - Changed the word invert all over the place to flip ;)
> - added icons for the horizontal and vertical flipping
Yay! Gotta love Github. :) I just looked through the diffs in your clone -
that's the way to go!
Thanks a lot!
> I didn't make any changes to rotation, because I was curious whether you
> didn't
> like the way I set it up with the paste. I know you made some
> suggestions for me
> to fix it, but it doesn't sound like you want it to work that way. Once
> you let
> me know, I'll fix up the rotation and commit it.
Agreed. I'm sending you the explanation on how selection rotation should
work in the other email.
> Also, I just realized you tried adding me to yahoo chat and I rejected
> you,
> sorry :-X What's your e-mail so I can add you?
No worries. :)
My YID is r0b0_d3s1gn2. Change 0 to o, 3 to e, and 1 to i.
> Also, I don't feel like you are being hard on me with the advice, I
> appreciate
> everything you're doing and I agree with most of your recommendations.
> Especially using the max function in the Math class, forgot it existed ;)
Great!
Thanks again for your code contribution. I can't wait to merge it into the
main code. :)
Best regards,
Mihai
Le Sun, 03 Oct 2010 23:49:46 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> I'll try to get myself set up on the code base today. Thank you for
> setting this
> up :)
You're welcome!
> The last patch I did doesn't include the rotation of the marquee at all.
> I
> actually don't like the idea of rotating the marquee, as it creates
> unintuitive
> situations for the user and poses difficulty when adding new features to
> the
> selection tool. What do you think of my alternative to just having the
> ability
> to paste rotated content, instead?
>
> I have to look into your notes from before, but it looks like I can just
> use the
> bufferContext to do the rotation.
The way I believe selection rotation should work is as follows:
- you make the rectangular selection.
- you pick the "rotate" option (from the toolbar), to enable rotation
mode. This button should be disabled if image manipulation is off.
- you then click on either of the four corners and drag to rotate the
selection. You rotate the selected pixels and the marquee as well.
- when the user is done (by pressing Enter, or by dropping the selection,
or by disabling rotation mode), the selected pixels stay rotated, *but*
the marquee's rotation is reset to 0, and the dimensions are updated such
that the selected pixels are enclosed in the selection (your
rotationDimensions() method allows this).
- this will allow the user to nicely and intuitively scale the rotated
image. keeping the marquee rotated would be, as you say, unintuitive.
What do you think? Does this sound fine for you?
> Cheers, thanks again for the changes to the repository!
Thank YOU!
Best regards,
Mihai
Le Mon, 04 Oct 2010 16:15:28 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> What you've suggested sounds good to me :)
Great! Also, your idea of having the two keys [ and ] for rotation is a
really good one. Keep this in the upcoming patch!
> I'll get on it! I'm pretty busy today, but I'll aim to have this done by
> Wednesday.
Thanks!
> Hey Mihai,
>
> Quick question. How would you recommend I implement enabling/disabling
> the
> rotation button when the transform button is clicked? Is there an event
> I can
> use to handle this or would I have to create one from scratch?
You just add the click event handler, nothing custom per-se. You change
the button class to app.gui.classPrefix + '_toolActive' (for the button)
when the rotation mode is active.
Today I had the chance to look over your code and test it. It's awesome,
selection rotation is done. We just need to iron out the user experience.
Le Fri, 08 Oct 2010 17:45:06 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> I haven't had as much time to work on this as anticipated (as well, I'm
> still
> trying to familiarize myself with your code). I'll have more time this
> weekend,
> but I've uploaded the changes I've made so far to my git repository. I
> believe
> the the widthOriginal, heightOriginal problem is resolved, too.
Don't worry about the time. I very much appreciate the work you are doing!
Regarding the widthOriginal and heightOriginal problem, I commented in
github.
> Right now, I've created a sel.rotate option. I'm trying to decide
> whether this
> would be better as a command or a configuration option. I was thinking
> that
> perhaps it would be better to make this a command and it lets you rotate
> the
> selection canvas around the canvas' origin from any point of the parent
> canvas
> if you use the command, as opposed to just the corners. As with programs
> like
> GIMP and photoshop, rotation itself is considered a single action.
I think it should be a command, as you say. And yes, when selectionMode is
true, then the user can rotate the pixels starting from any location, not
just the corners.
> Also, I'm thinking about using a custom cursor of a circle that has an
> arrow to
> indicate rotation (right now I'm using the wait cursor, lol), do you
> have any
> suggestions for how I might want to implement that? Right now, when the
> source
> code is compiled, it generates base64 formats. I'm wondering if doing
> something
> like that should go into the selection.js file directly or be placed
> elsewhere.
> Or, perhaps you have a better idea? I thought about using the standard
> directional keys to indicate rotation, too. Like if you were at the
> "northeast"
> corner, it would use southwest cursor.
What you did in the latest commit is really good. I think that the cursor
itself should be an actual image file. You can reference it using:
.paintweb_selectionMarquee.rotationCursor {
cursor: url("icons/rotation-cursor.png");
}
(put this in style.css)
In the mouseAreaUpdate() function just use cursor = '' (no cursor, no data
URI). Instead, update the marquee element className to include
rotationCursor. That will make the cursor we want to show.
During make the cursor image will be read and transformed into base64
encoding.
> Anyhow, sorry for the time it's taking. I've tried implementing this a
> bunch of
> different ways now ;)
Again, no need to apologize! You're doing great, and I very much
appreciate your work!
Thanks a lot for your work! I'll review later today the changes you did in
the latest commit. ;) At a quick glance: the changes are exactly in the
line with what I suggested. Kudos to you!
Le Sat, 09 Oct 2010 23:34:45 +0300, Gordon Clark <ucl...@yahoo.com> a
écrit:
> Hey Mihai,
>
> I made another commit today, which I think is a lot closer to how we
> want the
> tool to be. I am just using the "progress" cursor to indicate the
> rotation. I
> made it so in rotation mode you can rotate the marquee by clicking
> anywhere on
> it. I had done it before so you could rotate the marquee around anywhere
> on the
> canvas but it created problems with the cursor. Also, disabling rotation
> mode
> after the person is done rotating is a bit tedious because it's not a DRY
> solution for when the user presses keydown.
The commit you pushed works quite better, indeed. Also, I agree, it's a
bit tedious to drop the selection / merge changes once the user completes
the rotation. Anyhow, it's much better than not to have the rotation
option at all! ;)
> I know I'm kind of moving away from some of the suggested functionality
> you've
> asked for in terms of building this out. I'm trying to avoid overhauling
> the
> entire tool to cater to this one feature. But, if you are set on the
> previous
> method you described, I can work on it. I think it will just take a lot
> more
> time and there are other features I'd like to build in which I think
> would be
> easier to make.
Entirely agreed. We don't want to rewrite the selection tool code for
this, obviously. :)
Here's what I suggest: you read the two reviews I submitted today, for
both commits and see what is easy to fix from the things I said. Don't go
for the hard stuff. Move on to other features you want to build. I want
you to contribute code that you *like*, I want you to have *fun* first and
foremost! Don't get bored with the harder stuff. :)
Next weekend I'll take the selection code you'll have by then, and see
what I can do to finalize it, and get it merged into the main PaintWeb
code. Your contribution will be credited!
Keep updating me when you have changes for new features as well, because I
want to help you with suggestions, ideas and all you need. Just let me
know! Please don't mind my replies are a bit slow.
Thank you again!
Best regards,
Mihai