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.

Sign up for GitHub

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

Jump to bottom

Adds a grid to the flowgraph canvas. #6822

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Adds a grid to the flowgraph canvas. #6822

wants to merge 2 commits into from

Conversation

sdh11
Copy link
Contributor

@sdh11 sdh11 commented Sep 8, 2023

Description

This adds a grid to the flowgraph canvas which can be optionally shown by the user.

Related Issue

Fixes #5672.

Which blocks/areas does this affect?

GRC

Testing Done

Did basic testing to make sure the feature worked as expected.

Checklist

@sdh11
grc: optionally show a grid on the flowgraph canvas
72092af 
Adds a menu option that allows users to optionally show/hide the grid.
Only enables 'lines'. 'Dots' and 'hybrid' are implemented but very slow.

Signed-off-by: Seth Hitefield <sdhitefield@gmail.com>
@sdh11 sdh11 requested review from dkozel and mormj September 8, 2023 18:09
@willcode
Copy link
Member

willcode commented Sep 8, 2023 •
edited

I think #5672 (despite the title) is also about snap-to-grid. Does this PR also snap?``

Ah, ok, it's the small snap grid that already exists.

@sdh11
Copy link
Contributor Author

sdh11 commented Sep 8, 2023

Yes, it's driven by the same points as snap to grid, so blocks should be aligned.

@willcode
Copy link
Member

willcode commented Sep 9, 2023

Updated Python formating on a couple of files using tools/autoformat_python.sh to make CI pass.

@willcode
Copy link
Member

willcode commented Sep 9, 2023 •
edited

Grid spacing is correct, but the blocks do not line up on the grid.
image
At some zoom levels, they do line up. At zoom 1 on my machine, they do not.

Also, with such a tight grid, do you find the lines useful?

@dkozel
Copy link
Contributor

dkozel commented Sep 15, 2023

Sounds like a possible additional issue would be to change the block rendering to round up to an integer of the snap grid size? That might improve visual consistency? IIRC the original feature request came from the general chat room and I posted it on their behalf. I do think the grid has some use (defaulting to off) and, aside from the commented out portion, has a small code footprint.

mbr0wn
mbr0wn reviewed Sep 21, 2023
View reviewed changes
grc/gui/DrawingArea.py
cr.move_to(0, h_i)
cr.line_to(width, h_i)
cr.stroke()
# elif grid_mode == "dots":
Copy link
Member

Choose a reason for hiding this comment

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

I think we should either leave this in, or delete these lines. I'm for leaving it in, even if it's not obvious how to change it (I don't think this PR needs to add ways to set this through the GUI, for example).

@willcode willcode marked this pull request as draft September 28, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbr0wn mbr0wn mbr0wn left review comments

@dkozel dkozel Awaiting requested review from dkozel

@mormj mormj Awaiting requested review from mormj

Assignees
No one assigned
Labels
Backport-3.10 Should be backported to 3.10 enhancement GRC Needs Action
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to display grid in GRC canvas
4 participants
@sdh11 @willcode @dkozel @mbr0wn