Add Scale widget

[can-lehmann]

Gtk's Scale widget is currently missing. See https://docs.gtk.org/gtk4/class.Scale.html

[PhilippMDoerner]

I've been taking a look at this out of curiosity:
I don't quite get how the hooks know how to connect signals.

For example here:

renderable Entry of BaseWidget:
  text: string
  placeholder: string ## Shown when the Entry is empty.
  width: int = -1
  maxWidth: int = -1
  xAlign: float = 0.0
  visibility: bool = true
  invisibleChar: Rune = '*'.Rune

  proc changed(text: string) ## Called when the text in the Entry changed
  proc activate() ## Called when the user presses enter/return

  hooks:
    beforeBuild:
      state.internalWidget = gtk_entry_new()
    connectEvents:
      proc changedCallback(widget: GtkWidget, data: ptr EventObj[proc (text: string)]) {.cdecl.} =
        let text = $gtk_editable_get_text(widget)
        EntryState(data[].widget).text = text
        data[].callback(text)
        data[].redraw()
      
      state.connect(state.changed, "changed", changedCallback)
      state.connect(state.activate, "activate", eventCallback)
    disconnectEvents:
      state.internalWidget.disconnect(state.changed)
      state.internalWidget.disconnect(state.activate)

Owlkettle knows based on the fact that procs were defined on the renderable that there's a changed and an activate "field" on the type which can be connected to the GTK signals so that these procs get called.

But what are "changedCallback" and "eventCallback" and how does one know when to use which?

The only pattern I somewhat recognize across the file is that you're apparently supposed to always write:

state.connect(state.<procName>, "<signal-name>", <procName>EventCallback

with procName in the above examples being changed and activate to the signal-names "changed" and "activate", though the entire eventCallback for activate kinda throws off the pattern. Does what I type as the last parameter just not matter?

[can-lehmann]

The last parameter refers to the proc that is passed directly to GTK for handling the event. Its job is to:

1. Read back any state changes from GTK
2. Call the owlkettle event handler (and pass any arguments it might have)
3. Call redraw to update the UI after the event was handled.

E.g. for the changed event, which is called when the user input some text, we need to read back the new text and update the Entry.text field with it. Then we call owlkettle's Entry.changed handler and finally we call redraw to update the UI. changedCallback is responsible for doing that.

Other events like activate are simpler, because we do not need to read back state & pass arguments to the owlkettle event handler. This is why there is also a general handler called eventCallback.

In case of the Scale widget, you will need to implement a custom valueChangedCallback, that actually reads back the new position of the scale.

[PhilippMDoerner]

How can you know what the signature of the callback for a given gpointer for a given signal is?
For example for the ColorButton it is defined:

void
color_set (
  GtkColorButton* self,
  gpointer user_data
)

You wrapped that as:

      proc colorSetCallback(widget: GtkWidget, data: ptr EventObj[proc (color: tuple[r, g, b, a: float])]) {.cdecl.} =
        var gdkColor: GdkRgba
        gtk_color_chooser_get_rgba(widget, gdkColor.addr)
        let color = (gdkColor.r.float, gdkColor.g.float, gdkColor.b.float, gdkColor.a.float)
        ColorButtonState(data[].widget).color = color
        data[].callback(color)
        data[].redraw()

So somehow you:

1. Figured out that gpointer in this context translates to ptr EventObj and
2. Figured out that the EventObj contains a proc with the signature proc (color: tuple[r, g, b, a: float])

How did you figure that out? Mostly relevant as similarly I need to figure out what's hidden behind the gpointer for valueChanged.

[can-lehmann]

The type of data is just a pointer to an EventObj that contains your owlkettle callback. Look at the definition of connect: https://github.com/can-lehmann/owlkettle/blob/main/owlkettle/widgetutils.nim#L56

[PhilippMDoerner]

Check. The way I currently see it I don't think it makes sense to wrap https://docs.gtk.org/gtk4/ctor.Scale.new.html as it requires a GtkAdjustment and I don't quite understand what to do with that one.

Wrapping https://docs.gtk.org/gtk4/ctor.Scale.new_with_range.html and using that for instantiation seems "correcter" to me, any thoughts?

[can-lehmann]

I would personally use gtk_scale_new and pass nil as the Adjustment (see docs), so that it creates its own adjustment internally. The min/max can then be set in the correct hooks via the methods the Range parent class.

[PhilippMDoerner]

Hmm I'll try that out once I've got something functional.
I've got a compileable example for scale at least. But it seems immutable and I can't seem to make it mutable. As in, the slider is frozen at the left-hand side of the scale.

I think this has to do with the Accessible Role: https://docs.gtk.org/gtk4/property.Accessible.accessible-role.html

I can't seem to find a way to define it initially, have you had to deal with that before?

[PhilippMDoerner]

Above you can see the first "stab" I've taken at the entire thing.
I've so far figured out that accessible relates to general a11y aka accessibility for screen readers etc. not accessing values.

I keep getting stuck at this state of the widget:

As you can see its greyed out. It does not move, ever. Similarly the changedValue proc never gets triggered.
But the build-hook for the marks field does trigger and show that echo $(state.internalWidget.gtk_widget_is_sensitive()).bool is true.
I don't know what's going wrong here.

[can-lehmann]

Are you sure that you set the min/max values for the slider / range? I would assume that if the interval has length 0, it might also trigger this state. You might also need to set a step/page size (if there is not special method for that, then using a method on the adjustemnt obtained via get_adjustment).

Edit: Sorry did not see the code in your fork. It looks like you set the min max, but not the step/page size.

[can-lehmann]

I just had a look at your branch and believe that I found the issue. It seems like your gtk bindings in gtk.nim are wrong. C's double type should be cdouble in nim. If you change the types at the appropriate locations, your code seems to work:

[PhilippMDoerner]

Yep, that was quite a dumb mistake 😆
Thanks for pointing that out, that helped quite a fair bit and got me the basics, now to figure out all the hooks etc.

[PhilippMDoerner]

Hmmm somewhat stuck on 2 separate problems now:

1. GTK is behaving super weirdly with the window if the scale has marks on it. Compile the example as it is right now and if you slide the slider around up to near the max value, the window will start expanding. No clue where the hack that is coming from or if that's a GTK bug. See the screencast for an example.

Screencast.from.2023-09-27.22-12-51.webm

2. I can't seem to update currentValue properly. I want to have a field that is accessible from the outside that always depicts the current value of the scale. But I don't quite get why, it just doesn't update the value. It always remains the value of "currentValue" that the parent-widget passes on. Not sure how to fix that one.

Screencast.from.2023-09-27.22-18-02.webm

[can-lehmann]

For 2., that is expected given that you set currentValue = 0.5 in your example. It is always immediately reverted to 0.5 after being updated in the event handler. Since you also don't have a hook for currentValue, the GTK widget is never updated to reflect the owlkettle widget state.

[PhilippMDoerner]

Ahhh right. Okay, got an update and a build-hook now, which means I do get initial values.

I am not entirely sure if the hooks updating all three of GTK (via gtk_range_set_value) and the state (via valueChangedEventCallback where it is set via ScaleState(data[].widget).currentValue = scaleValue) and the widget (via update-hook, widget.valCurrentValue = state.currentValue) is the right way to go, but it works for now.

  hooks currentValue:
    build:
      gtk_range_set_value(state.internalWidget, widget.valCurrentValue)
      echo "Build: ", widget.valCurrentValue, " - ", state.currentValue
    update:
      gtk_range_set_value(state.internalWidget, state.currentValue) # This seemed more correct as state.currentValue keeps being the one thing that 
      widget.valCurrentValue = state.currentValue # Necessary as otherwise widget.valCurrentValue remains at 0.5, meaning out of sync with the value in state
      echo "Update: ", widget.valCurrentValue, " - ", state.currentValue

I think looking back having a "currentValue" field might have been the wrong approach for what I wanted.
What I want is for the user to be able to access the currentValue of a Widget whenever they want (e.g. the Scale might be used inside a Form and when somebody clicks the Okay button you want to extract the values of your individual input widgets).
Having a currentValue field I thought might've been the correct move, but I think in fact what I want is an initialValue field and after that a proc getValue(): float64 that you can call on the Scale-Widget to call gtk_range_get_value.

Is that doable? Or is that even the right approach?

[PhilippMDoerner]

I might be facing an actual problem with the Scale being actually "2 way" in its bindings.
By that I mean, I want there to be a field on the Scale widget called value (previously called "currentValue").

1. If I update value from the application, the value as it is depicted in the rendered widget should update to reflect that.
2. If the user uses the widget to update the value, then the value in the widget-state should be updated.

However, 1) requires me to have an update field-hook on the value field which updates the widget-state with the value within the value field, like so:

  hooks value:
    update:
      echo "Updated value to ", widget.valValue
      gtk_range_set_value(state.internalWidget, widget.valValue)

If I do that though, then changes to the widget will be impossible because they also trigger the update field-hook on value during the redraw and that will override the value-change from the user using the widget.

I can't seem to figure that one out. You can provide an initial value, but programmatically after that basically only the user can update the value =/

[PhilippMDoerner]

Welp, I managed to fix the "window expands infinitely" issue at least:
Whenver the update-hook of "Marks" was called, it added new marks, but never cleared them.
So basically after the update hook ran 4 times, I told GTK to add 16 marks in total.
By changing the hook to include state.internalWidget.gtk_scale_clear_marks() like so:

  hooks marks:
    (build, update):
      state.internalWidget.gtk_scale_clear_marks()
      let hasScaleMarks = widget.hasMarks and widget.valMarks.len > 0
      if hasScaleMarks:
        for mark in widget.valMarks:
          let label: string = if mark.label.isSome(): mark.label.get() else: $mark.value
          gtk_scale_add_mark(state.internalWidget, mark.value , mark.position.toGtk(), label.cstring)

It fixes the issue

[can-lehmann]

I might be facing an actual problem with the Scale being actually "2 way" in its bindings.

Have a look at how it works for Entry. The text field just has a property hook that updates the GTK.Entry's text. In your example code, the valueChanged callback needs to update the value in app, so that a two way binding actually exists.

On a more general note: You should have property hooks for all fields of the widget, not handle them in beforeBuild. The hook for marks can also be a property hook.

[PhilippMDoerner]

Okay, I think I got most functionality that is configurable from Range and Scale itself covered.
Now I could also go through GtkWidget, GObject, GtkAccessible and GtkOrientable to look for more set procs that could be configured via properties on Scale.

Is that within the scope of the ticket?
Do you maybe have a list of properties for any of those that any renderable widget should cover?
Basically, where should I draw the line?

Off the cuff, I could make out just as an overview the following function that might be of interest and possibly worth wrapping:

GTK_Accessible:

• gtk_accessible_set_accessible_parent
• gtk_accessible_update_next_accessible_sibling
• gtk_accessible_update_relation
• gtk_accessible_update_relation_value
• gtk_accessible_update_state
• gtk_accessible_update_state_value

GTK.Orientable:

• gtk_orientable_set_orientation

GTK.Widget:

• gtk_widget_set_default_direction
• gtk_widget_action_set_enabled
• gtk_widget_activate
• gtk_widget_activate_action
• gtk_widget_activate_action_variant
• gtk_widget_activate_default
• gtk_widget_add_controller
• gtk_widget_add_css_class
• gtk_widget_add_tick_callback
• gtk_widget_add_mnemonic_label
• gtk_widget_insert_action_group
• gtk_widget_mnemonic_activate
• gtk_widget_remove_css_class
• gtk_widget_remove_tick_callback
• gtk_widget_remove_mnemonic_label
• gtk_widget_set_can_target
• gtk_widget_set_can_focus
• gtk_widget_set_direction
• ... and way, way more

[can-lehmann]

Since you inherit from BaseWidget, all relevant fields from GtkWidget should already be available (in case anything should be missing, that is probably a separate PR). Since you have a orient field, adding a property hook for it that uses gtk_orientable_set_orientation makes sense. To be honest, I have not looked into GTK's accessability APIs yet, so since they are currently not supported by any other widget, adding support is definitely out of scope for this PR.

I think that feature wise, this issue is mostly complete (other than the orient field). Please open a PR, so we can discuss the remaining steps.

[PhilippMDoerner]

This issue thus closed?

[can-lehmann]

Yes 😄