Setting pixel data may not always do the right thing

This is a bit of a mix of an issue and a question. I'm building some basic photo editing, and canvas is obviously a good fit for this in elm/the web. My design relies on me being able to draw on top of an image to show crop/selection regions. With the current implementation, it appears you're copying/drawing by setting pixels on the canvas - is there a good reason for this? This prevents me from drawing transparencies on the image, and seems inefficient for most drawing operations. I get that pixel operations are basically the lowest primitive we can get, but why not leverage more of the native functions? I haven't done any benchmarking, so I don't know if there are any performance benefits, but I'd think it would at least be on par with your current by-pixel implementation, with the added bonus of being able to draw on top of images with transparency.

Locally, I'm working on an implementation that I've described using more canvas 2d methods, and I'd be happy to refactor it and write a PR for you if you're interested.

I am not sure I follow.

So you say 'it appears youre copying/drawing by setting pixels on the canvas - is there a good reason for this?'. I am not sure what you mean. Some of the operations in Canvas.js are setting individual pixels with ctx.putImageData() (setPixel, setPixels, drawLine, drawRectangle, and fromImageData). Other functions are not setting individual pixels; Canvas.drawCanvas and Canvas.drawImage rely on ctx.drawImage().

And you say 'this prevents me from drawing transparencies', but as I understand things, ctx.drawImage() will factor in transparency. Since Canvas.drawCanvas and Canvas.drawImage boil down to ctx.drawImage() they should factor in transparency.

You ask 'why not leverage more native functions?'. You mean, why do Canvas.drawRectangle and Canvas.drawLine rely on Canvas.setPixels instead of ctx.drawLine() and ctx.drawRect()? I made things that way because the native functions anti-alias quite a lot, and I didnt want elm-community/canvas to assume thats what the programmer wanted. The whole point is to give basic and low level control of the canvas element, and its a lot easier to go from pixel-perfect lines to anti-aliased lines than vice versa.

Anyway, thats just where I am coming from! I am positive things could be improved. Maybe you could tell me a bit more about what you are trying to do. What do you think? Yes, PRs are welcome.

First, I forgot to say thank you for implementing a pretty neat canvas library. My writing style is sort of blunt, so I'm sorry if I sounded overly critical.

So my main issue is that I don't want to have to create a new canvas to draw a semi-transparent rectangle over my original image. I'm sure canvas does some optimization and skips plotting rgba(0,0,0,0) pixels while doing a drawImage, but it will still have to iterate over all the pixels to do this. The canvas line function lets me draw a semi-transparent line, rect, polygon, etc directly on my canvas, but if you use setImageData, it will not blend, it will just over-write those pixels.

My secondary issue is doing canvas copying to keep the data immutable is a huge performance penalty. In my case, I'm doing touch events on the canvas, and in the chrome browser, you have up to 200ms to prevent the default browser drag behaviour (scroll, back button, etc.), but if I continually do these immutable steps, I have >200ms delay and my prevent default does not fire. In my local copy of canvas, I stripped out the immutable-by-default behaviour, and instead have an opt-in Canvas.Native.copy : Canvas -> Canvas method that can be called at the beginning of a single re-render. This occasionally is still a performance hit (my images are 350x180, so not huge by any means), but at least I can avoid doing copies during touch interaction. That's all to say, opt-in immutable got me from stuttered/bugged dragging of images to very smooth near-native feel.

Also, you can disable anti-aliasing/smoothing, and design that to be opt-in, too.

As far as the branch/mutilation of your code I've been working on, you can peek at a snapshot here. I'm just implementing the things I need as I go, so I've stripped out some features you had or changed how they work entirely (ie loadImage loads directly to a canvas). If I were to re-implement most of the features you have, are you interested in a PR from this? It would be a joy to merge 💩 .

Hey Alex,

0 Transparent Rectangle I never thought of that. I had to look up the stack overflow on how to draw a transparent rectangle. (http://stackoverflow.com/questions/10487882/html5-change-opacity-of-a-draw-rectangle). Currently in elm-community/canvas, one would type..

drawTransparentRectangle : Color -> Size ->  Position -> Canvas -> Canvas
drawTransparentRectangle size transparentColor =
  Canvas.initialize size
  |> Canvas.fill transparentColor
  |>Canvas.drawCanvas

.. which I guess is verbose. It sounds like youd prefer something native like...

drawRectangle : Color -> Size -> Position -> Canvas -> Canvas

.. that does all that automatically?

I will think about this more. Presently, its hard to imagine following this path without making either many different functions that draw rectangles, or making a drawRectangle function that takes a ton of parameters. Perhaps it makes sense to have a Canvas library with some basic functions, and then Canvas.Extra with additional functions like drawSolidRectangle : Color -> Size -> Position -> Canvas -> Canvas

1 Immutability Right, there is a performance drop from making things immutable. I didn't really notice much of a problem, but I also didn't do any bench marking and I never supposed my case would be representative of everyone's. When I made things mutable, lots of funny things happened. For example, one could not pass a canvas to multiple toHtml functions. Making mutable Elm functions seems like a deal breaker, Elm just doesnt work the way it should be when things are mutable.

That said, there has got to be another way. You correctly note that within Native/Canvas.js there is a lot canvas copying going on. Every time a canvas goes to toHtml its copied, and every time its drawn on its copied. I dont know how to not-copy and maintain immutability, but certainly it could be optimized. What if instead we made a type called DrawOp and drawing functions returned DrawOp instead of Canvas -> Canvas. To use a DrawOp one runs it through Canvas.batch : List DrawOp -> Canvas -> Canvas.

So for example...

 Canvas.batch [ Canvas.drawCanvas redSquare (Position 0 0), Invert ] mainCanvas

would replace

mainCanvas
  |> Canvas.drawCanvas redSquare (Position 0 0)
  |> Invert

With Canvas.batch in the example above, mainCanvas would be copied once, rather than twice.

2 Loading images directly to Canvas

Hey that makes a lot of sense. There is no sense of having a separate Image type if the only thing that can be done with it is pasted onto a Canvas and converted into a Canvas.

3 Moving forward

Let me think about this more. Let me run some tests to see how much performance is lost by not using native functions. Let me verify that CanvasRenderingContext2D.imageSmoothingEnabled = False; truly does turn off anti-aliasing.

Just investigated imageSmoothingEnabled, and as far as I can tell it doesnt get rid of anti aliasing. That was what I recall from using imageSmoothingEnabled. Not sure what its supposed to do if not that, so it seems plausible to me that I am just not using it correctly. Here is a screen shot of my test. dank meme 2017-02-02 at 3 38 36 pm dank meme 2017-02-02 at 3 38 58 pm

Having them in the view params is probably worse, because we potentially lose having a Canvas model, which is bad for Canvas.drawImage and Canvas.loadImage. Inversely, Canvas.batch as you've described it should just do the right thing, and we know at the end of all the operations, we can do a single copy to make the canvas immutable.

In fact, with draw operations, that may also resolve the rectangle drawing issue, being we definitely don't want a drawFilledRect, drawRect, drawFilledBorderRect, etc. If we stuck with draw ops, we could just do:

  Canvas.batch
    model.canvas
    [ SetFill "#A0A0A0"
    , DefineRect 0 0 100 100
    , Fill
    , SetStroke "#ff00ff"
    , DefineRect 0 0 100 100
    , Stroke
    ]

Just have a big union type for all the canvas operations (and maybe a few custom ones, like an immutable clone):

type DrawOp
  = SetFillStyle String
  | SetFont String
  | SetLineCap String
  | SetLineDashOffset Float
  | SetLineWidth Float
  | SetMiterLimit Float
  | SetShadowBlur Int
  | SetShadowColor String
  | SetShadowOffset Int Int
  | SetStrokeStyle String
  | SetTextAlign String
  | SetBaseLine String
  | Arc Point Float Float Float Bool
  | ArcTo Point Point Float
  | BeginPath
  | BezierCurveTo Point Point Point
  | ClearRect Point Size
  | ClosePath
  | DrawImage Canvas ( Point, Size ) ( Point, Size )
  | Fill
  | LineTo Point
  | MoveTo Point
  | PutImageData List Int Point
  | QuadraticCurveTo Point Point
  | Rect (Point, Size)
  | Save
  | Restore
  | SetLineDash List Int
  | EnableImageSmoothing String -- same as `context.imageSmoothingEnabled = true; context.imageSmoothingQuality = param`
  | DisableImageSmoothing -- same as `context.imageSmoothingEnabled = false`
  -- For real sickos
  | SetTransform Int Int Int Int Int Int -- Replace transform `context.setTransform()`
  | MultTransform Int Int Int Int Int Int -- Multiply transform `context.transform()`
  -- Back to normal people stuff
  | Stroke
  | StrokeText String Point Int
  | Translate Int Int

I skipped any method that was combining two other methods (ie strokeRect and fillRect), methods that may not make sense (ie drawWidgetAsOnScreen), or methods that aren't batchable (ie createImageData, createRadialGradient, etc.).

I think we definitely have something here, and I'd be willing to put a little time into making this a thing, which will unfortunately completely break compatibility, but I think solves a huge performance problem and maintains immutability.

What do you think?

Hey Alex,

That is an awesome idea! Lets do it!

How do we get started?

I am gonna push elm-community/canvas to a new tag called 0.1.0 or something, start up a new branch called 0.2.0 that we could work off.

Well, let's plan out what we need to do, and maybe divide up the work.

I guess this is the general check list (maybe add a ROADMAP.md?):

[x] Canvas, Size, Position
[x] Canvas.init : Size -> Canvas
[x] Canvas.view : List (Attribute msg) -> Canvas -> Html msg (I'm going to side with this over toHtml, since view seems to be the community standard)
[x] Canvas.getSize : Canvas -> Size
[ ] Canvas.toDataURL : Canvas -> String
[x] Canvas.imageLoad : String -> Task Error Canvas
[ ] type DrawOp = ... (Well, I did make that list, but it's definitely up for discussion)
[ ] Canvas.batch : List DrawOp -> Canvas -> Canvas - we can break this task up into logical pieces, since that's going to be a really big case statement. I think with that, we would want to break it up into methods so the Canvas.batch method isn't huge.
[ ] Canvas.clone : Canvas -> Canvas (The code definitely exists, just need to refactor it into a callable method)
[ ] Maybe Canvas.Events, since mouse and touch events probably aren't the main concern of canvas, but would be nice to haves, and probably should be part of the package entry point
[ ] Maybe Canvas.ImageData, since it's a much lower level interface - we'd probably want to wrap it in some type data and give some helper read and write functions.
[ ] Unit testing. Haven't done unit tests in Elm, so we can go on a code adventure.

I see you're on the elm slack server, so we can arrange some other things from there (probably just trello ~~trello~~...just one trello is good enough).

Elm-Canvas / elm-canvas

Setting pixel data may not always do the right thing #10