Schmavery / reprocessing

ReasonML graphics library inspired by Processing
https://schmavery.github.io/reprocessing/
MIT License
682 stars 24 forks source link

Strokes don't transform properly #34

Closed ryanartecona closed 7 years ago

ryanartecona commented 7 years ago

Given this example program:

open Reprocessing;

let setup env => {
  Env.size width::600 height::400 env;
  ()
};

let draw () env => {
  let (width, height) = (Env.width env, Env.height env);
  Draw.background Constants.black env;
  Draw.stroke Constants.red env;
  Draw.fill Constants.blue env;
  Draw.translate x::150. y::150. env;
  Draw.rotate (Constants.pi /. 60.) env;
  Draw.rect pos::((-100), (-100)) width::200 height::200 env;
  ()
};

run ::setup ::draw ();

I get this output (in the browser, so using reason-web): image

It looks to me like the current transform is getting applied once for the fill and twice for the stroke? The center of the red stroke is twice as far away from the top left origin than the blue fill, and it's also rotated more.

Possibly related, if I don't have a stroke at all (e.g. comment out the Draw.stroke line from the above example), the corners of a rect fill get clipped awkwardly:

image

Not sure if this is intentional or not, but it's at least unexpected for me as a newcomer. I can make the clipping go away by setting the stroke width to 0, but if there is no stroke color by default, why does my fill get clipped?

ryanartecona commented 7 years ago

I might have time to look into this later myself if someone can point me in the right direction.

Schmavery commented 7 years ago

Ran out of time to fix this tonight but it should be pretty straightforward, looks like I'm applying the transformation twice on the stroke like you said.

For the second, you need to explicitly set Draw.noStroke env; to remove the stroke. Seems like the second image is displaying with a black-colored stroke by default. I was actually just thinking I should make the default noStroke, which might be more intuitive.

Thanks for reporting these issues! It also really helped to have the images explaining.

ryanartecona commented 7 years ago

Happy to help!

I think it's due to these lines: https://github.com/Schmavery/reprocessing/blob/bsb-support-new/src/Reprocessing_Draw.re#L151-L154. Draw.quadf calls out to Draw.linef for the stroke, but quadf gives linef already-transformed points, which linef transforms again. That also explains why the dots at the stroke vertices are transformed properly, since the calls to Internal.drawEllipse below don't apply the env's transform internally, so those ultimately only get transformed once.

I think making noStroke the default would be more intuitive for me, but I have no graphics background, so I don't know what the common practice is there for other libraries.

Schmavery commented 7 years ago

I made noStroke the default and fixed this transform bug. Lmk how it goes. One bonus is we got to stop drawing a few redundant ellipses, which are on the more expensive side 😄

ryanartecona commented 7 years ago

Thanks! Just verified this is working perfectly for me now.