Skip to content

Conversation

andrewvarga
Copy link
Contributor

@andrewvarga andrewvarga commented Jul 24, 2025

Fixes #1502

After discussing with mech. engineers and the frontend team, this PR adds new grid / snapping related settings.
There are two grid types: major grid is a darker grid, it has a few lighter grids inside.

Screenshot 2025-08-19 at 21 38 28
  • Major Grid Spacing: how much space to have between major grid lines [given in current base units]. Eg. if set to 2 and current base unit is mm, then there is 2mm between major grid lines.
  • Minor Grids Per Major: how many minor grid lines to have between major grid lines
  • Snap To Grid: on/off (this is added into the right-click ContextMenu too)
  • Snaps Per Minor: how many snaps to have for each minor grid.
  • Fixed size grid: this existed before for 3D but now it's also used for sketch mode

These are added as both user and project level settings, but let me know if that's not your preference.

I'm thinking if the already existing "Show Scale Grid" option that currently only applies to 3D should be connected to Sketch mode but it's tricky because it's off by default, but in 2D it should be on by default

This PR implements rendering the grid and snapping to the grid according to the above settings in sketch mode (3D not affected).
If the Fixed size grid option is true, the major and minor lines are rendered in their correct sizes, except if they become too small we don't render them anymore.
If that option is turned off, we dynamically render the most appropriate 10 multiple of the original size to show something meaningful in case the grids would become too large / too small.

It renders the grid as an infinite grid by rendering LINES with a custom shader that doesn't use geometry.
I did this to avoid recreating / upluading geometry buffers on each frame, or having a complicated pool/reuse logic of geometries, it's simpler to just calculate the line positions by vertexId.
Other options would be to render a full screen quad and create a custom fragment shader but it would be a bit more tricky to configure / sometimes this approach has platform dependent artifacts depending on shader precision.
The limitation is that these lines can only be 1 px thick, so if we need thicker lines this can be switched to render quads instead with the same shader trick, or use the full screen quad / fragment shader where thickness is more configurable.

I changed the UnitsMenu to show the current base unit scale true to size in Sketch mode, I thought it would be useful in general and especially when the grid is shown:
Screenshot 2025-08-19 at 21 44 50
Screenshot 2025-08-19 at 21 46 13
Screenshot 2025-08-19 at 21 46 34

In addition there are few small related/unrelated fixes about rendering, canvas resizing.
eg. previous grid was actually rendered on top of sketch segments which I assume is not on purpose so I fixed it (testing-settings needed to be updated because of that):
Screenshot 2025-09-01 at 17 37 00

Remaining issues:

  • Add crosshair for first click when snapping is active
  • grid rendering when starting on other axes
  • Feature pane collapsing bug reported by Max this seems to be unrelated to this branch

Things I have on my radar for next (would include in this PR but it's already too big):

  • X, Y axis should be infinite also
  • X, Y axis shouldn't disappear when switching from [mm] -> [feet]
  • enabled reset view / center camera in sketch mode because now with the infinite grid it's easier to lose the origin

Older description before I settled on the approach (ignore this):

I'm thinking we could have a dropdown of 3 options for snapping, instead of just an on/off boolean:

  • snapping off
  • snap to default units
  • snap to grid

That said, I have trouble imagining "snap to grid" being very useful with the current grid. On my screen these are the 2 extremes I have for the grid size:

  • Largest: only a few grids fit on the screen, snapping to the grid intersections would be pretty large jumps, doesn't feel like it would be useful. It's a similar to using default units and zooming in too much as you mentioned.
Image
  • Smallest possible grids: still only 10 grids fit my screen per row, so still a bit too big jumps. This could be improved by:
    • Changing the grid zoom steps to allow grids to be smaller
    • Make the grid infinite to cover the screen, then 2x more grids would be visible per row
Image

With those changes it could be useful to have the "snap to grid" option too, but I should change those things first then (?).

Related: am I correct that the "Fixed Size Grid" setting only applies to outside of sketch mode, not within sketch mode where it seems to have no effect? I wonder if that is on purpose or it should be fixed first too.

@andrewvarga andrewvarga requested review from a team as code owners July 24, 2025 17:30
Copy link

vercel bot commented Jul 24, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
modeling-app Ready Ready Preview Comment Sep 1, 2025 8:32pm

@andrewvarga andrewvarga marked this pull request as draft July 24, 2025 17:30
Copy link

codspeed-hq bot commented Jul 24, 2025

CodSpeed Instrumentation Performance Report

Merging #7893 will not alter performance

Comparing andrewvarga/1502/add-snap-to-grid-to-sketch-mode (6c56b73) with main (bcb6ed2)

Summary

✅ 89 untouched benchmarks

@pierremtb pierremtb linked an issue Jul 24, 2025 that may be closed by this pull request
2 tasks
@andrewvarga
Copy link
Contributor Author

Update from sync today:

  • Let the user set the grid size explicitly (eg. feet, 5 inches, 10mm), the grid being an infinite grid (current grid leaves empty space on the edge of the screen). The grid size is not dependant on current zoom level in the first pass of the implementation.
  • The user can snap to the grid, this grid snapping is lower priority than other snappings: tangent to line, tangent to start profile, main axes.
  • We can have a second, grayed out lower level grid which is dependend on the zoom level.

/// The space between major grid lines, specified in the current unit
#[serde(default, skip_serializing_if = "is_default")]
pub major_grid_spacing: f64,
/// Specifies ow many minor grid lines to have per major grid line.
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo in the field description for minor_grids_per_major. It should read "Specifies how many minor grid lines" instead of "Specifies ow many minor grid lines".

Suggested change
/// Specifies ow many minor grid lines to have per major grid line.
/// Specifies how many minor grid lines to have per major grid line.

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

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.

Add snap-to-unit-grid to sketch mode
2 participants