Piano roll grid menu

[swesterfeld]

This PR adds support for letting the user set the grid size manually. It supports tuplets (we could not do this at all before), so for instance you can uses 1/8 triplet grids. It also supports small grid sizes, while being smart enough to not draw too many grid lines.

I've preserved the code we already have for automatic grid size selection. I believe that this can be nice, because you can zoom in, and as you zoom in the grid adapts to smaller and smaller grids. So with the PR, I added an explicit choice between grid mode "auto" (automatic grid size) and "manual" (user defined grid size).

[swesterfeld]

As far as I know, you already have other code for computing grid lines (for the arranger) and you want my code and the other code be merged and shared between piano roll and arranger; so it would be great if you could provide your code.

[tim-janik]

@greptile

[greptile-apps]

Greptile Overview

Greptile Summary

This PR enhances the piano roll editor by adding a grid menu system that enables manual grid size selection with tuplet support. The key improvements include:

• Dual-mode grid system: Users can now choose between "auto" mode (existing zoom-adaptive behavior) and "manual" mode (user-defined grid sizes)
• Tuplet support: The system now handles complex rhythmic divisions like 1/8 triplets (2/3/5/7 tuplets), which was previously impossible
• Enhanced grid menu: A comprehensive UI with quantization options from 1/128 to2 measures, plus tuplet multipliers
• Smart grid rendering: The system intelligently limits grid line density to prevent visual clutter

The implementation refactors the quantization logic in piano-ctrl.js by extracting shared functionality into a standalone quantization() function that branches between auto and manual modes. In pianoroll.js, the changes include new grid state properties, menu handling methods, tuplet calculation functions, and a complete rewrite of the grid drawing algorithm in paint_timegrid(). This architecture preserves backward compatibility while enabling precise rhythmic control essential for advanced music editing workflows.

Important Files Changed

Changed Files

Filename	Score	Overview
ui/b/piano-ctrl.js	4/5	Refactored quantization system to support both automatic and manual grid modes with shared logic
ui/b/pianoroll.js	4/5	Added comprehensive grid menu system with tuplet support and rewrote grid rendering algorithm

Confidence score: 4/5

• This PR is safe to merge with low risk as it preserves existing functionality while adding new features
• Score reflects solid implementation with proper separation of concerns, though the grid rendering rewrite is complex and could benefit from additional testing
• Pay close attention to ui/b/pianoroll.js grid calculation logic, particularly the tuplet scaling in grid_length_factor() and the new grid drawing algorithm

Sequence Diagram

sequenceDiagram
    participant User
    participant PianoRoll as b-piano-roll
    participant GridMenu as pianogridmenu
    participant Layout as piano_layout
    participant Repaint as repaint

    User->>PianoRoll: "Click grid label"
    PianoRoll->>GridMenu: "popup(event)"
    GridMenu-->>User: "Show grid options menu"
    
    User->>GridMenu: "Select grid option (tuplet/quantization)"
    GridMenu->>PianoRoll: "activate event with uri"
    PianoRoll->>PianoRoll: "setgrid(uri)"
    
    alt Tuplet Selection (2, 3, 5, 7)
        PianoRoll->>PianoRoll: "grid_tuplet = parseInt(uri)"
    else Grid Length Selection
        PianoRoll->>PianoRoll: "grid_length = uri"
    end
    
    PianoRoll->>PianoRoll: "grid_stepping = PPQN * 4 / grid_length_factor()"
    PianoRoll->>PianoRoll: "calculate grid_label with tuplet suffix"
    PianoRoll->>PianoRoll: "setAttribute('grid_label', grid_label)"
    
    PianoRoll->>Layout: "updated() triggers piano_layout_()"
    Layout->>Layout: "calculate stepping based on grid_length and grid_tuplet"
    Layout-->>PianoRoll: "layout updated"
    
    PianoRoll->>Repaint: "queue_repaint()"
    Repaint->>Repaint: "paint_timegrid() with new stepping"
    Repaint-->>User: "Grid lines updated with new spacing"

Loading

_{2 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: Consider using strict equality (===) instead of loose equality (==) for consistency with the rest of the codebase

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/b/piano-ctrl.js
Line: 44:44

Comment:
**style:** Consider using strict equality (===) instead of loose equality (==) for consistency with the rest of the codebase

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: Should validate that piano_roll.grid_stepping exists and is a valid number before returning it to prevent potential runtime errors

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/b/piano-ctrl.js
Line: 51:51

Comment:
**logic:** Should validate that `piano_roll.grid_stepping` exists and is a valid number before returning it to prevent potential runtime errors

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: grid_stepping calculation only runs when setgrid() is called, but paint_timegrid() uses it for manual mode. If grid_stepping is 0 initially, manual mode may not work correctly until first menu selection.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/b/pianoroll.js
Line: 377:377

Comment:
**logic:** grid_stepping calculation only runs when setgrid() is called, but paint_timegrid() uses it for manual mode. If grid_stepping is 0 initially, manual mode may not work correctly until first menu selection.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

logic: Division by signature[0] could be zero if time signature is malformed, causing division by zero.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/b/pianoroll.js
Line: 858:858

Comment:
**logic:** Division by signature[0] could be zero if time signature is malformed, causing division by zero.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

style: Using setAttribute for reactive property updates instead of direct assignment may not trigger component re-rendering properly.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ui/b/pianoroll.js
Line: 384:384

Comment:
**style:** Using setAttribute for reactive property updates instead of direct assignment may not trigger component re-rendering properly.

How can I resolve this? If you propose a fix, please make it concise.