-
Notifications
You must be signed in to change notification settings - Fork 24
feat: UI work to facilitate future improvements in midi-into-track-input #183
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,5 +46,17 @@ func (l LabelStyle) Layout(gtx layout.Context) layout.Dimensions { | |
| } | ||
|
|
||
| func Label(str string, color color.NRGBA, shaper *text.Shaper) layout.Widget { | ||
| return LabelStyle{Text: str, Color: color, ShadeColor: black, Font: labelDefaultFont, FontSize: labelDefaultFontSize, Alignment: layout.W, Shaper: shaper}.Layout | ||
| return SizedLabel(str, color, shaper, labelDefaultFontSize) | ||
| } | ||
|
|
||
| func SizedLabel(str string, color color.NRGBA, shaper *text.Shaper, fontSize unit.Sp) layout.Widget { | ||
| return LabelStyle{ | ||
| Text: str, | ||
| Color: color, | ||
| ShadeColor: black, | ||
| Font: labelDefaultFont, | ||
| FontSize: fontSize, | ||
| Alignment: layout.W, | ||
| Shaper: shaper, | ||
| }.Layout | ||
|
Comment on lines
-49
to
+61
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm. not sure about this anymore, I added a specific need for the SizedLabel at one point, but I am not sure (it might be in the upcoming midi input PR though) |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package gioui | ||
|
|
||
| import "gioui.org/layout" | ||
|
|
||
| // general helpers for layout that do not belong to any specific widget | ||
|
|
||
| func EmptyWidget() layout.Spacer { | ||
| return layout.Spacer{} | ||
| } | ||
|
|
||
| func OnlyIf(condition bool, widget layout.Widget) layout.Widget { | ||
| if condition { | ||
| return widget | ||
| } | ||
| return EmptyWidget().Layout | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,8 +12,6 @@ import ( | |
| "gioui.org/op/paint" | ||
| "gioui.org/text" | ||
| "gioui.org/unit" | ||
| "gioui.org/widget" | ||
| "gioui.org/widget/material" | ||
| "github.com/vsariola/sointu/tracker" | ||
| ) | ||
|
|
||
|
|
@@ -103,26 +101,31 @@ func (m *MenuStyle) Layout(gtx C, items ...MenuItem) D { | |
| } | ||
| icon := widgetForIcon(item.IconBytes) | ||
| iconColor := m.IconColor | ||
| if !item.Doer.Allowed() { | ||
| iconColor = mediumEmphasisTextColor | ||
| } | ||
| iconInset := layout.Inset{Left: unit.Dp(12), Right: unit.Dp(6)} | ||
| textLabel := LabelStyle{Text: item.Text, FontSize: m.FontSize, Color: m.TextColor, Shaper: m.Shaper} | ||
| if !item.Doer.Allowed() { | ||
| // note: might be a bug in gioui, but for iconColor = mediumEmphasisTextColor | ||
| // this does not render the icon at all. other colors seem to work fine. | ||
| iconColor = disabledTextColor | ||
| textLabel.Color = mediumEmphasisTextColor | ||
|
Comment on lines
+107
to
110
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was weird behaviour: other than the code suggested, if iconColor was set to mediumEmphasisTextColor, it really displayed no icon at all if not Allowed(). with my upcoming menu (for midi vel track selection), I really want the possibility to show an icon on a disabled menu item. this change makes it possible. |
||
| } | ||
| shortcutLabel := LabelStyle{Text: item.ShortcutText, FontSize: m.FontSize, Color: m.ShortCutColor, Shaper: m.Shaper} | ||
| shortcutInset := layout.Inset{Left: unit.Dp(12), Right: unit.Dp(12), Bottom: unit.Dp(2), Top: unit.Dp(2)} | ||
| dims := layout.Flex{Axis: layout.Horizontal, Alignment: layout.Middle}.Layout(gtx, | ||
| layout.Rigid(func(gtx C) D { | ||
| return iconInset.Layout(gtx, func(gtx C) D { | ||
| p := gtx.Dp(unit.Dp(m.IconSize)) | ||
| p := gtx.Dp(m.IconSize) | ||
|
Comment on lines
-119
to
+117
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IDE told me this conversion was a noop
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IDE is right :) I also noticed that the language server started to make this kinds of suggestions (didn't do that earlier), just didn't have time to clean everything up yet. |
||
| gtx.Constraints.Min = image.Pt(p, p) | ||
| if icon == nil { | ||
| return D{Size: gtx.Constraints.Min} | ||
| } | ||
|
Comment on lines
+119
to
+121
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, popup Menu Items had to have an icon (IconBytes) for now, now these are optional |
||
| return icon.Layout(gtx, iconColor) | ||
| }) | ||
| }), | ||
| layout.Rigid(textLabel.Layout), | ||
| layout.Flexed(1, func(gtx C) D { return D{Size: image.Pt(gtx.Constraints.Max.X, 1)} }), | ||
| layout.Flexed(1, func(gtx C) D { | ||
| return D{Size: image.Pt(gtx.Constraints.Max.X, 1)} | ||
| }), | ||
| layout.Rigid(func(gtx C) D { | ||
| return shortcutInset.Layout(gtx, shortcutLabel.Layout) | ||
| }), | ||
|
|
@@ -168,14 +171,14 @@ func PopupMenu(menu *Menu, shaper *text.Shaper) MenuStyle { | |
| } | ||
| } | ||
|
|
||
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *widget.Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { | ||
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { | ||
| for clickable.Clicked(gtx) { | ||
| menu.Visible = true | ||
| } | ||
| m := PopupMenu(menu, tr.Theme.Shaper) | ||
| return func(gtx C) D { | ||
| defer op.Offset(image.Point{}).Push(gtx.Ops).Pop() | ||
| titleBtn := material.Button(tr.Theme, clickable, title) | ||
| titleBtn := Button(tr.Theme, clickable, title) | ||
| titleBtn.Color = white | ||
| titleBtn.Background = transparent | ||
| titleBtn.CornerRadius = unit.Dp(0) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,11 +158,8 @@ func (te *NoteEditor) layoutButtons(gtx C, t *Tracker) D { | |
| deleteTrackBtnStyle := ActionIcon(gtx, t.Theme, te.DeleteTrackBtn, icons.ActionDelete, te.deleteTrackHint) | ||
| splitTrackBtnStyle := ActionIcon(gtx, t.Theme, te.SplitTrackBtn, icons.CommunicationCallSplit, te.splitTrackHint) | ||
| newTrackBtnStyle := ActionIcon(gtx, t.Theme, te.NewTrackBtn, icons.ContentAdd, te.addTrackHint) | ||
| in := layout.UniformInset(unit.Dp(1)) | ||
| voiceUpDown := func(gtx C) D { | ||
| numStyle := NumericUpDown(t.Theme, te.TrackVoices, "Number of voices for this track") | ||
| return in.Layout(gtx, numStyle.Layout) | ||
| } | ||
| voiceUpDown := NumericUpDown(t.Theme, te.TrackVoices, "Number of voices for this track") | ||
| voiceUpDown.Padding = unit.Dp(1) | ||
| effectBtnStyle := ToggleButton(gtx, t.Theme, te.EffectBtn, "Hex") | ||
| uniqueBtnStyle := ToggleIcon(gtx, t.Theme, te.UniqueBtn, icons.ToggleStarBorder, icons.ToggleStar, te.uniqueOffTip, te.uniqueOnTip) | ||
| midiInBtnStyle := ToggleButton(gtx, t.Theme, te.TrackMidiInBtn, "MIDI") | ||
|
|
@@ -176,10 +173,10 @@ func (te *NoteEditor) layoutButtons(gtx C, t *Tracker) D { | |
| layout.Rigid(effectBtnStyle.Layout), | ||
| layout.Rigid(uniqueBtnStyle.Layout), | ||
| layout.Rigid(Label(" Voices:", white, t.Theme.Shaper)), | ||
| layout.Rigid(voiceUpDown), | ||
| layout.Rigid(voiceUpDown.Layout), | ||
| layout.Rigid(splitTrackBtnStyle.Layout), | ||
| layout.Flexed(1, func(gtx C) D { return layout.Dimensions{Size: gtx.Constraints.Min} }), | ||
| layout.Rigid(midiInBtnStyle.Layout), | ||
| layout.Rigid(OnlyIf(t.HasAnyMidiInput(), midiInBtnStyle.Layout)), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: what's wrong with If this place is the only reason to introduce EmptyWidget and OnlyIf, the total number of lines is far less, and it's immediately obvious what's going on here. |
||
| layout.Flexed(1, func(gtx C) D { return layout.Dimensions{Size: gtx.Constraints.Min} }), | ||
| layout.Rigid(deleteTrackBtnStyle.Layout), | ||
| layout.Rigid(newTrackBtnStyle.Layout)) | ||
|
|
@@ -287,15 +284,15 @@ func (te *NoteEditor) layoutTracks(gtx C, t *Tracker) D { | |
| c = cursorColor | ||
| } | ||
| if hasTrackMidiIn { | ||
| c = cursorForTrackMidiInColor | ||
| c = trackMidiInCurrentColor | ||
| } | ||
| te.paintColumnCell(gtx, x, t, c) | ||
| te.paintColumnCell(gtx, x, t, c, hasTrackMidiIn) | ||
| } | ||
| // draw the corresponding "fake cursors" for instrument-track-groups (for polyphony) | ||
| if hasTrackMidiIn { | ||
| if hasTrackMidiIn && y == cursor.Y { | ||
| for _, trackIndex := range t.Model.TracksWithSameInstrumentAsCurrent() { | ||
| if x == trackIndex && y == cursor.Y { | ||
| te.paintColumnCell(gtx, x, t, cursorNeighborForTrackMidiInColor) | ||
| if x == trackIndex { | ||
| te.paintColumnCell(gtx, x, t, trackMidiInAdditionalColor, hasTrackMidiIn) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -334,10 +331,10 @@ func (te *NoteEditor) layoutTracks(gtx C, t *Tracker) D { | |
| return table.Layout(gtx) | ||
| } | ||
|
|
||
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA) { | ||
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA, ignoreEffect bool) { | ||
| cw := gtx.Constraints.Min.X | ||
| cx := 0 | ||
| if t.Model.Notes().Effect(x) { | ||
| if t.Model.Notes().Effect(x) && !ignoreEffect { | ||
|
Comment on lines
+334
to
+337
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when paintColumnCell is called to highlight a cell background inanother track, it is useful to ignore that cell Effect value or one can only color the current Nibble() - usually not intended. (actually, only intended for the current cursor) |
||
| cw /= 2 | ||
| if t.Model.Notes().LowNibble() { | ||
| cx += cw | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ type NumericUpDownStyle struct { | |
| Tooltip component.Tooltip | ||
| Width unit.Dp | ||
| Height unit.Dp | ||
| Padding unit.Dp | ||
| shaper text.Shaper | ||
| } | ||
|
|
||
|
|
@@ -74,11 +75,19 @@ func NumericUpDown(th *material.Theme, number *NumberInput, tooltip string) Nume | |
| Tooltip: Tooltip(th, tooltip), | ||
| Width: unit.Dp(70), | ||
| Height: unit.Dp(20), | ||
| Padding: unit.Dp(0), | ||
| shaper: *th.Shaper, | ||
| } | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I do not specifically use the NumericUpDownPadded anymore (see above, I changed my mind), question is whether I should remove the change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Padding is still used, but I removed the wrapper calls to the constructor |
||
|
|
||
| func (s *NumericUpDownStyle) Layout(gtx C) D { | ||
| if s.Padding <= 0 { | ||
| return s.layoutWithTooltip(gtx) | ||
| } | ||
| return layout.UniformInset(s.Padding).Layout(gtx, s.layoutWithTooltip) | ||
| } | ||
|
|
||
| func (s *NumericUpDownStyle) layoutWithTooltip(gtx C) D { | ||
| if s.Tooltip.Text.Text != "" { | ||
| return s.NumberInput.tipArea.Layout(gtx, s.Tooltip, s.actualLayout) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,13 @@ import ( | |
| "gioui.org/op/clip" | ||
| "gioui.org/op/paint" | ||
| "gioui.org/unit" | ||
| "gioui.org/widget" | ||
| "github.com/vsariola/sointu/tracker" | ||
| "github.com/vsariola/sointu/version" | ||
| "golang.org/x/exp/shiny/materialdesign/icons" | ||
| ) | ||
|
|
||
| type SongPanel struct { | ||
| MenuBar []widget.Clickable | ||
| MenuBar []Clickable | ||
|
||
| Menus []Menu | ||
| BPM *NumberInput | ||
| RowsPerPattern *NumberInput | ||
|
|
@@ -57,7 +56,7 @@ type SongPanel struct { | |
|
|
||
| func NewSongPanel(model *tracker.Model) *SongPanel { | ||
| ret := &SongPanel{ | ||
| MenuBar: make([]widget.Clickable, 3), | ||
| MenuBar: make([]Clickable, 3), | ||
| Menus: make([]Menu, 3), | ||
| BPM: NewNumberInput(model.BPM().Int()), | ||
| RowsPerPattern: NewNumberInput(model.RowsPerPattern().Int()), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,8 +60,10 @@ var activeLightSurfaceColor = color.NRGBA{R: 45, G: 45, B: 45, A: 255} | |
| var cursorColor = color.NRGBA{R: 100, G: 140, B: 255, A: 48} | ||
| var selectionColor = color.NRGBA{R: 100, G: 140, B: 255, A: 12} | ||
| var inactiveSelectionColor = color.NRGBA{R: 140, G: 140, B: 140, A: 16} | ||
| var cursorForTrackMidiInColor = color.NRGBA{R: 255, G: 100, B: 140, A: 48} | ||
| var cursorNeighborForTrackMidiInColor = color.NRGBA{R: 255, G: 100, B: 140, A: 24} | ||
|
|
||
| var trackMidiInCurrentColor = color.NRGBA{R: 255, G: 100, B: 140, A: 48} | ||
| var trackMidiInAdditionalColor = withScaledAlpha(trackMidiInCurrentColor, 0.7) | ||
| var trackMidiVelInColor = withScaledAlpha(trackMidiInCurrentColor, 0.3) | ||
|
|
||
| var errorColor = color.NRGBA{R: 207, G: 102, B: 121, A: 255} | ||
|
|
||
|
|
@@ -75,3 +77,13 @@ var dialogBgColor = color.NRGBA{R: 0, G: 0, B: 0, A: 224} | |
|
|
||
| var paramIsSendTargetColor = color.NRGBA{R: 120, G: 120, B: 210, A: 255} | ||
| var paramValueInvalidColor = color.NRGBA{R: 120, G: 120, B: 120, A: 190} | ||
|
|
||
| func withScaledAlpha(c color.NRGBA, factor float32) color.NRGBA { | ||
| A := factor * float32(c.A) | ||
| return color.NRGBA{ | ||
| R: c.R, | ||
| G: c.G, | ||
| B: c.B, | ||
| A: uint8(A), | ||
| } | ||
| } | ||
|
Comment on lines
+80
to
+89
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unclear whether such a helper function should live in this specific file, but I found that every existing color-morphing-function inside gioui package was not exported (or I did not look correctly)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, they were uneager to export them. Apparently, there's better color handling functions in "out there" (according to them), so they don't want to maintain public API for the color functions. But I don't know what that "out there" could be; probably meant some library (or is there something in the standard library)? Anyway, I didn't look into this more deeply. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,9 +101,7 @@ func NewTracker(model *tracker.Model) *Tracker { | |
|
|
||
| func (t *Tracker) Main() { | ||
| titleFooter := "" | ||
| w := new(app.Window) | ||
| w.Option(app.Title("Sointu Tracker")) | ||
| w.Option(app.Size(unit.Dp(800), unit.Dp(600))) | ||
| w := NewWindow() | ||
| t.InstrumentEditor.Focus() | ||
| recoveryTicker := time.NewTicker(time.Second * 30) | ||
| t.Explorer = explorer.NewExplorer(w) | ||
|
|
@@ -127,9 +125,7 @@ func (t *Tracker) Main() { | |
| } | ||
| if !t.Quitted() { | ||
| // TODO: uh oh, there's no way of canceling the destroyevent in gioui? so we create a new window just to show the dialog | ||
| w = new(app.Window) | ||
| w.Option(app.Title("Sointu Tracker")) | ||
| w.Option(app.Size(unit.Dp(800), unit.Dp(600))) | ||
| w = NewWindow() | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is only a de-duplication here... |
||
| t.Explorer = explorer.NewExplorer(w) | ||
| go eventLoop(w, events, acks) | ||
| } | ||
|
|
@@ -165,6 +161,13 @@ func (t *Tracker) Main() { | |
| t.quitWG.Done() | ||
| } | ||
|
|
||
| func NewWindow() *app.Window { | ||
| w := new(app.Window) | ||
| w.Option(app.Title("Sointu Tracker")) | ||
| w.Option(app.Size(unit.Dp(800), unit.Dp(600))) | ||
| return w | ||
| } | ||
|
|
||
| func eventLoop(w *app.Window, events chan<- event.Event, acks <-chan struct{}) { | ||
| // Iterate window events, sending each to the old event loop and waiting for | ||
| // a signal that processing is complete before iterating again. | ||
|
|
@@ -344,3 +347,10 @@ func (t *Tracker) removeFromMidiNotePlaying(note byte) { | |
| } | ||
| } | ||
| } | ||
|
|
||
| func (t *Tracker) HasAnyMidiInput() bool { | ||
| for _ = range t.Model.MIDI.InputDevices { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ type UnitEditor struct { | |
| CopyUnitBtn *TipClickable | ||
| ClearUnitBtn *ActionClickable | ||
| DisableUnitBtn *BoolClickable | ||
| SelectTypeBtn *widget.Clickable | ||
| SelectTypeBtn *Clickable | ||
| commentEditor *Editor | ||
| caser cases.Caser | ||
|
|
||
|
|
@@ -47,7 +47,7 @@ func NewUnitEditor(m *tracker.Model) *UnitEditor { | |
| ClearUnitBtn: NewActionClickable(m.ClearUnit()), | ||
| DisableUnitBtn: NewBoolClickable(m.UnitDisabled().Bool()), | ||
| CopyUnitBtn: new(TipClickable), | ||
| SelectTypeBtn: new(widget.Clickable), | ||
| SelectTypeBtn: new(Clickable), | ||
| commentEditor: NewEditor(widget.Editor{SingleLine: true, Submit: true}), | ||
| sliderList: NewDragList(m.Params().List(), layout.Vertical), | ||
| searchList: NewDragList(m.SearchResults().List(), layout.Vertical), | ||
|
|
@@ -236,9 +236,9 @@ func (pe *UnitEditor) command(e key.Event, t *Tracker) { | |
| type ParameterWidget struct { | ||
| floatWidget widget.Float | ||
| boolWidget widget.Bool | ||
| instrBtn widget.Clickable | ||
| instrBtn Clickable | ||
| instrMenu Menu | ||
| unitBtn widget.Clickable | ||
| unitBtn Clickable | ||
| unitMenu Menu | ||
| Parameter tracker.Parameter | ||
| tipArea component.TipArea | ||
|
|
@@ -332,7 +332,6 @@ func (p ParameterStyle) Layout(gtx C) D { | |
| gtx.Constraints.Min.Y = gtx.Dp(unit.Dp(40)) | ||
| instrItems := make([]MenuItem, p.tracker.Instruments().Count()) | ||
| for i := range instrItems { | ||
| i := i | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not a lol, it's a bug fix. But you are right, Golang 1.22 fixed this, so it's not needed anymore. See: https://go.dev/blog/loopvar-preview
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh. really?? sorry, that just looked so far beyond anything having real use, but I'll be the first to admit having way too little knowledge about golang or its history...
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's a bit silly, that's why golang people changed the language semantics. Interestingly, same mistake in language design was made by C#, see https://learn.microsoft.com/en-us/archive/blogs/ericlippert/closing-over-the-loop-variable-considered-harmful. And they also changed the semantics of the language so you don't have to do But so far, this has been the only breaking change in v1 of go; disregarding this, all old go code since v1 still compiles & should work. Why language designers make this mistake? My guess is that it's because at early stages of their language design, their compiler is not very good at optimizing, and |
||
| name, _, _, _ := p.tracker.Instruments().Item(i) | ||
| instrItems[i].Text = name | ||
| instrItems[i].IconBytes = icons.NavigationChevronRight | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make Enabled() state visible on ToggleButtons