aullman / opentok-whiteboard

Shared Whiteboard that works with OpenTok
MIT License
34 stars 22 forks source link

Line smoothing with paper.js #21

Closed bostondv closed 8 years ago

bostondv commented 8 years ago

This PR expands on the work from #18 and should fix many of its issues.

In my testing smoothing has worked well in demo.html as well as when running it with 2 participants in opentok meeting.

Any feedback or testing would be appreciated!

bostondv commented 8 years ago

@aullman issues you pointed out should now be fixed! :)

aullman commented 8 years ago

Sorry, one more issue. Linking to it here because it's being buried in an outdated diff. https://github.com/aullman/opentok-whiteboard/pull/21#discussion_r51371186

bostondv commented 8 years ago

So I changed the draw, undo and redo methods quite a bit to fix that race condition, store the paper.js path object in the history and look up path's by their ID in the undo and redo methods.

Appears to be working well except for one issue... About 100px of the bottom and right sides of the canvas render the paths differently than the rest and don't clear when undoing or clearing entire canvas. It can't reproduce it on http://meet.tokbox.com but I have no idea what is causing it. Any thoughts?

Screenshot before and after an undo. Notice the line rendering on the right side and that same area remains after an undo.

screenshot-localhost 3005 2016-02-01 21-48-50

screenshot-localhost 3005 2016-02-01 21-48-56

bostondv commented 8 years ago

FYI seems to me the issue is related to paper.js handling of canvas size changes and there are some tickets like https://github.com/paperjs/paper.js/issues/549 that may be related. I've tried a number of hacks, but no luck yet...

bostondv commented 8 years ago

I believe the issue has been fixed - there were a few things related to the canvas / paper.js view size that needed to be changed.

aullman commented 8 years ago

I'm seeing some strange behaviour around the undo operation. Try doing this:

  1. Draw a line
  2. Draw a second line
  3. Click undo to undo the second line
  4. Draw a 3rd line
  5. Open up the whiteboard in a second window

Result: The second window can see all 3 lines.

bostondv commented 8 years ago

Was able to reproduce the issue. I didn't consider the case that when you draw a new line the redoStack gets cleared and thus fails to hide paths that have been set to invisible when new connections are being setup.

I've replaced looping through redoStack for new connections with looping through the pathStack, finding any invisible paths and sending an undo signal for those paths.

The fix worked for me following your steps, however the paths to be hidden were visible for a split second to the new client between the moment of drawing the entire history and then hiding the invisible paths...

bostondv commented 8 years ago

Ended up adding a UUID to each path and drawHistory update as well as setting the visibility on each drawHistory update when undo's are done so that new connections can initialize the paths and set their visibility immediately as they are created.

aullman commented 8 years ago

Cool, thanks @bostondv this LGTM. I'm going to just give it one last test run through :)

aullman commented 8 years ago

Awesome, works great. Thanks so much for the contribution!

aullman commented 8 years ago

I just published this as v1.0.0

bostondv commented 8 years ago

Awesome, thanks for the help with it! :tada: