TreeSelection getSelected + iterNext is broken

[Dankr4d]

Hi,
It looks like, the gtk (+ dependency) update broke some more, but I'm not sure, since I didn't test this for a while. In my last stable version of my software it still works (didn't recompile the old version).

In following example I used a modified version of your listview example. Adding 10 rows, selecting the first row (index 0), calling getSelected with TreeSelection and then calling iterNext.

Example code:

import gintro/[glib, gobject, gtk]
import gintro/gio except ListStore

const
  LIST_ITEM = 0
  N_COLUMNS = 1

var list: TreeView

# we need the following two procs for now -- later we will not use that ugly cast...
proc typeTest(o: gobject.Object; s: string): bool =
  let gt = g_type_from_name(s)
  return g_type_check_instance_is_a(cast[ptr TypeInstance00](o.impl), gt).toBool
proc listStore(o: gobject.Object): gtk.ListStore =
  assert(typeTest(o, "GtkListStore"))
  cast[gtk.ListStore](o)


proc selectNext() =
  var iter: TreeIter
  var store: ListStore = listStore(list.getModel())
  if not list.selection.getSelected(store, iter):
    return
  if store.iterNext(iter):
    list.selection.selectIter(iter)
    list.scrollToCell(store.getPath(iter), nil, false, 0.0, 0.0)


proc initList(list: TreeView) =
  let renderer = newCellRendererText()
  let column = newTreeViewColumn()
  column.title = "List Item"
  column.packStart(renderer, true)
  column.addAttribute(renderer, "text", LIST_ITEM)
  discard list.appendColumn(column)
  let gtype = gStringGetType() # typeFromName("gchararray")
  let store = newListStore(N_COLUMNS, cast[ptr GType]( unsafeaddr gtype)) # cast due to bug in gtk.nim
  list.setModel(store)


proc appActivate(app: Application) =
  let
    window = newApplicationWindow(app)
    sw = newScrolledWindow()
  window. title = "List view"
  window.position = WindowPosition.center
  window.borderWidth = 10
  window.setSizeRequest(370, 270)
  list = newTreeView()
  sw.add(list)
  sw.setPolicy(PolicyType.automatic, PolicyType.automatic)
  sw.setShadowType(ShadowType.etchedIn)
  list.setHeadersVisible(false)
  window.add(sw)
  initList(list)

  var val: Value
  var iter: TreeIter
  let store = listStore(getModel(list))
  let gtype = gStringGetType()
  discard init(val, gtype)
  for idx in 0..10:
    setString(val, $idx)
    store.append(iter)
    store.setValue(iter, LIST_ITEM, val)
    if idx == 0:
      list.selection.selectIter(iter)

  showAll(window)
  selectNext()


proc main =
  let app = newApplication("org.gtk.example")
  connect(app, "activate", appActivate)
  discard run(app)

main()

Expected behavior:

It should select the second line of treeview.

Exception:

Hint: /home/dankrad/Desktop/liststoretest  [Exec]
Traceback (most recent call last)
/home/dankrad/Desktop/liststoretest.nim(78) liststoretest
/home/dankrad/Desktop/liststoretest.nim(76) main
/home/dankrad/.nimble/pkgs/gintro-#head/gintro/gio.nim(31423) run
/opt/Nim/lib/core/macros.nim(559) connect_for_signal_cdecl_activate1
/home/dankrad/Desktop/liststoretest.nim(70) appActivate
/home/dankrad/Desktop/liststoretest.nim(24) selectNext
/home/dankrad/.nimble/pkgs/gintro-#head/gintro/gtk.nim(18927) iterNext
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Error: execution of an external program failed: '/home/dankrad/Desktop/liststoretest '

[StefanSalewski]

Thanks for reporting, I will try to investigate that issue soon. v0.9.4 may have intraduced some changes maybe. Listview and treeview is always a bit fragile, first examples contained some casts, which is not really good. I learned about the listview stuff more than ten years ago from the book of A. Krause, but forgot most. That is the reason that my GTK4 book contains still no listview and treeview stuff, maybe I will manage to add that in the coming winter.

[StefanSalewski]

Do you really think that your code has worked before:

proc selectNext() =
  var iter: TreeIter
  var store: ListStore = listStore(list.getModel())
  if not list.selection.getSelected(store, iter):
    return
  if store.iterNext(iter): # line 24 with crash
    list.selection.selectIter(iter)
    list.scrollToCell(store.getPath(iter), nil, false, 0.0, 0.0)

Can iterNext() really work when iter is still uninitialled? Well, I would guess it can default to the first element of the store, but I would not rely on it, Will see if there is something like iterFirst(). Or have you seen C code where gtk_tree_model_iter_next() is used in that way?

References:

https://docs.gtk.org/gtk3/method.TreeModel.iter_next.html

[StefanSalewski]

OK, that seems not to be the core bug, as store is already nil. Have to investigate further.

[StefanSalewski]

proc getSelected*(self: TreeSelection; model: var (TreeModel | TreeModelSort | ListStore | TreeModelFilter | TreeStore) = cast[var TreeModel](nil);
    iter: var TreeIter = cast[var TreeIter](nil)): bool =
  var tmpoutgobjectarg: ptr TreeModel00
  result = toBool(gtk_tree_selection_get_selected(cast[ptr TreeSelection00](self.impl), cast[var ptr TreeModel00](if addr(model) == nil: nil else: tmpoutgobjectarg), iter))
#  dothemagic(model
  if addr(model) != nil:
    model = nil

  if tmpoutgobjectarg != nil:
    let argqdata = g_object_get_qdata(tmpoutgobjectarg, Quark)
    if argqdata != nil:
      model = cast[type(model)](argqdata)
      assert(model.impl == tmpoutgobjectarg)
    else:
      fnew(model, gtk.finalizeGObject)
      model.impl = tmpoutgobjectarg
      GC_ref(model)
      if g_object_is_floating(model.impl).int != 0:
        discard g_object_ref_sink(model.impl)
      g_object_add_toggle_ref(model.impl, toggleNotify, addr(model[]))
      g_object_unref(model.impl)
      assert(g_object_get_qdata(model.impl, Quark) == nil)
      g_object_set_qdata(model.impl, Quark, addr(model[]))

That proc is the problem, that proc has changed in the last 18 months. Sets store to nil, which maybe it should not do. Is is a complicated proc, as it has a var gobject parameter. I may have to fix it? Maybe you just use it in a wrong way, it is not that easy.

https://docs.gtk.org/gtk3/method.TreeSelection.get_selected.html

[Dankr4d]

Ok, I just read the documentation. I should have do this earlier (sorry to waste your time), but didn't expect that it also sets the model/store. It's the first time a gtk function modifies it (since I'm using gtk).

[...]
model is filled with the current model as a convenience
[...]
A pointer to set to the GtkTreeModel, or NULL.
The argument will be set by the function.
The argument can be NULL.
[...]

---

Maybe you just use it in a wrong way, it is not that easy.

I did, you're right.

How to use it correctly for my case:

  if not list.selection.getSelected(cast[var TreeModel](nil), iter):
    return

---

Do you really think that your code has worked before:

It worked before. I never touched this line for 2 years. For me it looks like it was a bug in gintro, but is solved now (I'm not sure).

---

Thanks again for your help and keeping this project alive.

[StefanSalewski]

I did, you're right

I am not really sure. Seems that you have found a way to avoid that issue, but I will investigate that proc in more detail later. Just wrote an issue to gnome forum, new API docs are not that great: https://discourse.gnome.org/t/new-api-docs-and-gtk-tree-selection-get-selected/7667

Will investigate later...

[Dankr4d]

Regarding to this phrase in documentation:

This function will not work if you use selection is #GTK_SELECTION_MULTIPLE.

You can set the treeview to single select or multi select (rows). If set to multiple selection, then the function cannot set the iter for the current selection, since there may be more rows selected.

[StefanSalewski]

It seems that gtk_tree_selection_get_selected() call in the bindings set the passed tmpoutgobjectarg argument not to a valid value, but to nil. So it is indeed not an obvious bindings bug, but more GTK related. But from API docs I would expect that function to set tmpoutgobjectarg to a valid none nil value when return value is true.

[StefanSalewski]

It is a Nim issue. The cast[var ptr TreeModel00] does not work as expected with the recent Nim compiler version. I guess I have to consult the Nim devs.

[Dankr4d]

Regarding to Nim version, I'm still on version 1.4.x, but also upgraded from 1.4.8 to 1.4.9.

[dankrad@TNB ~]$ nim --version
Nim Compiler Version 1.4.9 [Linux: amd64]
Compiled at 2021-09-28
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 5bed375b5298fb5977650d21dc6d63c86ecfe096
active boot switches: -d:release

[StefanSalewski]

Thanks for reporting your Nim version, I had some hope that 1.4.x versions would work at least still.

Have just created a minimal example for the Nim compiler issue, see

nim-lang/Nim#18924

So maybe it will be fixed. If they should refuse to fix, a larger gintro modification will be necessary.

[StefanSalewski]

Should be fixed now. The fix is indeed tiny:

$ diff ~/gintrotest/tests/gen.nim gen.nim 
3c3
< # v 0.9.5 2021-OCT-01
---
> # v 0.9.5 2021-SEP-29
1325c1325
<             result.arglist.add("cast[var " & ngr.name00 & "](if addr(" & name & ") == nil: nil else: addr tmpoutgobjectarg)")
---
>             result.arglist.add("cast[var " & ngr.name00 & "](if addr(" & name & ") == nil: nil else: tmpoutgobjectarg)")
3311a3312

and indeed at some locations in the gen.nim files code of that shape was used already before. But it is a bit strange still, and it is hard to remember how it has to look.

Your initial program seems to compile and work now.

Below is a modified version. I have added to gintro a typeCheckInstanceIsA() overload for Gobject parameter, which simplifies low level type checks. Initial parameter was only TypeInstance, which is useless. But then I found out that we do not need proc listStore() for type conversion, as Nim type test with (o of gtk.ListStore) and type conversion with gtk.ListStore(o) works. So we can use ListStore(gobject.Object(list.getModel))

Unfortunately a getSelected() with named parameters using the nil default for model does not compile, the problem may be overload resolution for that case. So we have to pass a casted nil or we have to pass an actual model.

import gintro/[glib, gobject, gtk]
import gintro/gio except ListStore

const
  LIST_ITEM = 0
  N_COLUMNS = 1

var list: TreeView

# As the safe type conversion works in our case, we do not need this proc at all
proc listStore(o: gobject.Object): gtk.ListStore =
  assert(typeCheckInstanceIsA((o), gtkListStoreGetType())) # low level gobject test
  assert(typeCheckInstanceIsA((o), g_type_from_name("GtkListStore"))) # same
  assert(o of gtk.ListStore) # test if the Nim proxy objects match
  return gtk.ListStore(o) # in this case we can do a safe Nim type conversion
  #assert(typeTest(o, "GtkListStore"))
  #cast[gtk.ListStore](o)

proc selectNext() =
  var iter: TreeIter
  var store: ListStore = ListStore(gobject.Object(list.getModel)) # listStore(list.getModel()
  #if not getSelected(self = list.getSelection, model = cast[var ListStore](nil), iter = iter): # nil default for model does not work, problem is overload resolution
  if not list.selection.getSelected(store, iter):
    return
  if store.iterNext(iter):
    list.selection.selectIter(iter)
    list.scrollToCell(store.getPath(iter), nil, false, 0.0, 0.0)

proc initList(list: TreeView) =
  let renderer = newCellRendererText()
  let column = newTreeViewColumn()
  column.title = "List Item"
  column.packStart(renderer, true)
  column.addAttribute(renderer, "text", LIST_ITEM)
  discard list.appendColumn(column)
  let gtype = gStringGetType() # typeFromName("gchararray")
  let store = newListStore(N_COLUMNS, cast[ptr GType](unsafeaddr gtype)) # cast due to bug in gtk.nim, we really should fix that!
  list.setModel(store)

proc activate(app: Application) =
  let
    window = newApplicationWindow(app)
    sw = newScrolledWindow()
  window.title = "List view"
  window.position = WindowPosition.center
  window.borderWidth = 10
  window.setSizeRequest(370, 270)
  list = newTreeView()
  sw.add(list)
  sw.setPolicy(PolicyType.automatic, PolicyType.automatic)
  sw.setShadowType(ShadowType.etchedIn)
  list.setHeadersVisible(false)
  window.add(sw)
  initList(list)

  var val: Value
  var iter: TreeIter
  let store = ListStore(gobject.Object(list.getModel)) # listStore(getModel(list))
  let gtype = gStringGetType()
  discard init(val, gtype)
  for idx in 0 .. 10:
    setString(val, $idx)
    store.append(iter)
    store.setValue(iter, LIST_ITEM, val)
    if idx == 0:
      list.selection.selectIter(iter)
  window.showAll
  selectNext()

proc main =
  let app = newApplication("org.gtk.example")
  app.connect("activate", activate)
  discard run(app)

main()

[Dankr4d]

Sorry for the late response. Thanks for the type conversion info, I'll change it in my application.
I currently didn't test your fix with Nim 1.4.9, but I'm sure it will work. In my case I don't need to pass the store anymore, since I only need the get the current iter (if I understood the gtk docu correctly).

Thanks for your work.