Skip to content

Align both brush and pen drawing in draw-ellipse - #26

Closed
lexi-lambda wants to merge 1 commit into
racket:masterfrom
lexi-lambda:fix-draw-ellipse-alignment
Closed

Align both brush and pen drawing in draw-ellipse#26
lexi-lambda wants to merge 1 commit into
racket:masterfrom
lexi-lambda:fix-draw-ellipse-alignment

Conversation

@lexi-lambda

Copy link
Copy Markdown
Member

For some reason, draw-ellipse goes out of its way to not align coordinates when drawing with the brush, but that doesn’t make a lot of sense. Not only is it weird and inconsistent, it can cause problems where the stroke would actually get drawn inside the boundary of the ellipse, leaving part of the fill peeking out from “under” the edge. Here’s a highly zoomed-in example:

Screen Shot 2020-06-04 at 14 28 31

Here, I’ve drawn a circle with a white fill and a grey stroke, superimposed on top of a black line. You can see that the edge of the fill is not fully covered by the stroke, so there’s an extra white fringe.

This PR just aligns both sets of coordinates, which is more consistent and fixes the problem. Here’s the same example with this change:

Screen Shot 2020-06-04 at 14 37 12

For some reason, draw-ellipse went out of its way to *not* align
coordinates when drawing with the brush, but that doesn’t make a lot of
sense. Not only is it weird and inconsistent, it could cause problems
where the stroke would actually get drawn inside the boundary of the
ellipse, leaving part of the fill peeking out from “under” the edge.

This just aligns both sets of coordinates, which is more consistent and
fixes the problem.
@lexi-lambda
lexi-lambda requested a review from mflatt June 4, 2020 19:37
@mflatt

mflatt commented Jun 4, 2020

Copy link
Copy Markdown
Member

I think it's not just draw-ellipse. All brush-based drawing avoids alignment, right?

I forget why things work the way they do, and I'll try to remember.

@lexi-lambda

lexi-lambda commented Jun 5, 2020

Copy link
Copy Markdown
Member Author

All brush-based drawing avoids alignment, right?

Ah, it seems like you’re right—I hadn’t even thought to check! I didn’t gather that from the documentation, though rereading it again, it doesn’t contradict what you’re saying, either.

The trouble going on here really only happens because I’m drawing to a pdf-dc%, so I can zoom in far enough in the result to see the misalignment. Perhaps the better fix here is to change slideshow to use 'smoothed instead of 'aligned when printing? After all, it seems like 'aligned doesn’t make much sense unless you are rasterizing.

I’d still like to understand why strokes are aligned but fills are not, but I’d be satisfied with an amendment to the documentation.

@mflatt

mflatt commented Jun 8, 2020

Copy link
Copy Markdown
Member

When I try

(slide
 (ct-superimpose
  (colorize (filled-ellipse 100 200) "black")
  (colorize (ellipse 100 200) "white")))

then it looks bad to me in 'aligned mode with or without the patch.

Consistent alignment seems to require (lambda (x) (align-x/delta x 0)) and (lambda (y) (align-y/delta y 0)) for the brush half, because that avoids a pen-width adjustment.

My best guess at the explanation for the current state: it took me a long time to sort out the pen-width correction (along the same lines as your patch for drawing dots), and older drawing backends used to have different rules. Long enough ago, trying to align functions for pen and brush drawing produced worse results, either because the alignment functions still weren't right or the backend was uncooperative.

Although I'm reluctant to change drawing calculations that have been in place a long time, it's worth exploring whether using brush alignment (the right one, consistently) would be better than the current state. Are you interested in exploring that more? If so, along with whatever else you look at, tests/racket/draw/draw may be useful for detecting differences.

@lexi-lambda

Copy link
Copy Markdown
Member Author

Although I'm reluctant to change drawing calculations that have been in place a long time, it's worth exploring whether using brush alignment (the right one, consistently) would be better than the current state. Are you interested in exploring that more?

What you’ve said makes sense to me. I may be interested in exploring that further, especially if I run into further issues, but it is admittedly a larger undertaking than I’m likely to pursue right now.

I have thought a little more about this issue since your previous comment, and the question I asked still seems relevant: would it make sense to simply not use 'aligned drawing in vector drawing contexts? My understanding of the 'aligned drawing mode is that it is intended to take advantage of knowledge of the size of the pixel grid, and in those scenarios, the issue is significantly less troublesome, since the error is guaranteed to be smaller than a single physical drawing unit. But in vector contexts, there is no pixel grid, so 'aligned doesn’t seem to make sense.

Doing something like changing the default alignment mode of ps-dc%, pdf-dc%, and svg-dc% might be overly drastic; it’s difficult to predict how that might affect existing programs. But changing slideshow’s printing mode to use 'smoothed seems like it might be more conservative. Does that seem reasonable to you?

@mflatt

mflatt commented Jun 8, 2020

Copy link
Copy Markdown
Member

Using 'smoothed for printing sounds ok to me.

@lexi-lambda

Copy link
Copy Markdown
Member Author

Alright. I’m going to close this PR, since either way, it doesn’t seem right. I’ll experiment with the slideshow change, and I may open a PR there if it seems to work out.

@lexi-lambda lexi-lambda closed this Jun 8, 2020
@lexi-lambda
lexi-lambda deleted the fix-draw-ellipse-alignment branch June 8, 2020 23:07
lexi-lambda added a commit to lexi-lambda/slideshow that referenced this pull request Jun 8, 2020
Using the 'aligned alignment mode doesn't make sense when printing,
since the PS/PDF output is scalable, and the pixel grid it will be
rasterized to is not known. Also see the discussion in racket/draw#26.
lexi-lambda added a commit to racket/slideshow that referenced this pull request Jun 10, 2020
Using the 'aligned alignment mode doesn't make sense when printing,
since the PS/PDF output is scalable, and the pixel grid it will be
rasterized to is not known. Also see the discussion in racket/draw#26.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants