Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'dot' line mode #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add 'dot' line mode #2

wants to merge 2 commits into from

Conversation

ybhvf
Copy link

@ybhvf ybhvf commented Apr 29, 2022

This commit adds a new 'dot' line mode which produces small dots via very short lines. This is useful for plotting dithered art.

This commit adds a new 'dot' line mode which produces small dots via
very short lines. This is useful for plotting dithered art.
@abey79
Copy link
Owner

abey79 commented May 2, 2022

Thanks for your contribution!

This is an interesting mode, however I don't understand the rationale behind using the PIXEL_OFFSET multiplier. This offset of 5 is related to the big mode, where each pixel is made of a 5x5 pen widths square. If the intent is indeed to spread out the points, it probably should be a command argument.

Also:

  • Tiny circles of diameter ~1-2 tenth of pen widths yield better result than small lines with most pen. I think it would be better for this mode (and I should likely do it every time a single pixel is drawn in the other modes btw).
  • Do you have any examples to contribute for the doc?

@ybhvf
Copy link
Author

ybhvf commented May 3, 2022

Hah, there's no reason behind my using the PIXEL_OFFSET multiplier... It's leftover from big_mode(), which I copied and modified for dot_mode().

I've removed PIXEL_OFFSET from dot_mode(), and have updated it to create circle paths instead. I'll also add an example later today :)

)

paths = [vp.circle(i, j, pen_width * 0.05)
for i, j in zip(indice_j, indice_i)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read as follows, for correctness and clarity:

paths = [vp.circle(j * pen_width, i * pen_width, pen_width * 0.05)
                  for j, i in zip(indice_j, indice_i)]

In particular, the * pen_width is needed such that the grid is indeed scaled by the pen width.

@abey79
Copy link
Owner

abey79 commented May 16, 2022

FYI, I made some changes that will probably need you to rebase and force-push.

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