jakubfiala / atrament

A small JS library for beautiful drawing and handwriting on the HTML Canvas.
http://fiala.space/atrament/demo
MIT License
1.56k stars 115 forks source link

Demo: Fill mode locks up browser (Chome & Firefox tested) #5

Closed bradmartin closed 8 years ago

bradmartin commented 8 years ago

fillbroken

If I get some time I'll run some profiling to see what's happening. When you change to fill mode in Chrome it will lock up the browser. It's not real evident but in the gif after it locks (shows spinner) I'm trying to click the controls, slide the range inputs with no responsiveness.

Great little lib by the way :+1:

jakubfiala commented 8 years ago

Hi, thanks for letting me know! Does this happen on both Chrome and FF then? What versions of both are you using? Which OS?

You might have noticed that this feature was added literally a few moments ago, so it's possible there are still bugs. However, I can't replicate this on Chrome or FF. Also, since it happens immediately after you switch the mode, it's probably not related to the filling algorithm itself, which is good.

bradmartin commented 8 years ago

Yea I noticed the update. It happens on both, FireFox does return the "unresponsive script" error dialog. ffdemo

Windows 10 Chrome = Version 49.0.2623.112 m FireFox = 43.0.1 && 44.0.2 && 45.0.2 (it's been awhile since I've used FF so I tested, updated, tested and updated each version :smile: )

jakubfiala commented 8 years ago

Hmm I've got the same versions, too, so this must be a platform-specific issue. Unfortunately I don't have access to a PC. Would you be able to build the lib from the source, and perhaps do some console logs to see where it breaks?

If not, I can get hold of a PC at some point, it just won't be today I'm afraid.

Also, do you not actually click on the canvas when it happens? It seems very unlikely that the script would freeze by just setting the mode variable.

bradmartin commented 8 years ago

I'll try to run the source and do some logging and profiling if possible. Might be later today. I'll let you know if I find anything :+1:

Just changing the mode seems to lock it up.

ptkach commented 8 years ago

I checked it on PC in Chrome and FF and if you try to fill whole canvas JS thread locks UI for a 5-7 seconds. In Mac it's faster ~ 3-4 sec.

@jakubfiala I reverted my local version to commit I did first on bucket filling and in Chrome on Mac it takes kinda 0.5 sec for a whole canvas filling, on PC ~ 1sec.

Will go deeper to understand what change makes it lag

jakubfiala commented 8 years ago

@ptkach that's good to know. In the future we'll be able to pass ImageData to a Web Worker so the JS thread doesn't freeze at all, but who knows when that will get 100% browser support.

One thing I did change in my modifications was that each colorPixel operation also performs two extra matchColor checks and potentially colors the two neighboring pixels, so that could be part of the problem.

In any case, what @bradmartin is experiencing seems like a different issue.

ptkach commented 8 years ago

I think it makes sense to check with my changes. Now it should be several times faster (< 1sec on PC).

jakubfiala commented 8 years ago

@bradmartin #6 is now merged to both master and gh-pages, so you should be able to test this problem with the demo.

bradmartin commented 8 years ago

Much better :+1: