Skip to content
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

Type introspection in gintro #205

Open
scott-asdfasdf opened this issue Aug 13, 2022 · 12 comments
Open

Type introspection in gintro #205

scott-asdfasdf opened this issue Aug 13, 2022 · 12 comments

Comments

@scott-asdfasdf
Copy link

Hi,
thanks for your interesting and useful wrapper.
I've been experimenting with a port of numberstation a python GTK app for MFA.

I'm struggling to find a good way of replicating this code from original:

def apply_css(self, widget, provider):
        Gtk.StyleContext.add_provider(widget.get_style_context(),
                                      provider,
                                      Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION)
        if isinstance(widget, Gtk.Container):
            widget.forall(self.apply_css, provider)

My attempt is something like this:

proc applyCSS(widgetPtr: ptr Widget00, providerPtr: pointer) {.cdecl.} =
  var widget: Widget
  new(widget)
  widget.impl = widgetPtr
  widget.ignoreFinalizer = true
  var provider = cast[CssProvider](providerPtr)
  widget.getStyleContext().addProvider(provider, STYLE_PROVIDER_PRIORITY_APPLICATION)

  if isContainer(widgetPtr) > 0:
    forall(cast[Container](widget), applyCSS, cast[pointer](provider))

This seems to work, but isContainer() is implemented in C and uses the macro IS_CONTAINER() because I'm not sure if there's a way to do type introspection through gintro?

I have also seen this issue, so my issue may be asking the same question?

@StefanSalewski
Copy link
Owner

Note that we should generally not use casts, the 00 types or access the .impl fields directly when we use the gintro bindings. The examples in the README and in the GTK4 book generally avoids this. Cdecl is used internally in the gintro macros, when a proc is called directly from GTK, as for the callback functions. The applyCSS() function looks more like an ordinary function called from Nim code, not like a callback, so cdecl may be not needed. For the container test, may something like "if widget of gtk4.Container" work? Applying forall may be a problem still, which may need an ugly cast. Generally the GTK4 book or the README should have an example for use of a CSS provider. Maybe you can use as parameter type for applyCSS() Container instead of Widget?

I was not able to see the whole Python code, I clicked on that git repository, but have not managed to navigate. So I don't know how large and complex the whole GUI part is. Is it GTK4 or still old GTK3?

Also note, that the fate of gintro is unclear. Mr. Rumpf may find new tricks to break its working for Nim 2.0, and actually the number of really active gintro users is close to zero, so I am considering removing it from github end of this years.

@scott-asdfasdf
Copy link
Author

scott-asdfasdf commented Aug 14, 2022

Thanks for your detailed response.

Note that we should generally not use casts, the 00 types or access the .impl fields directly when we use the gintro bindings. The examples in the README and in the GTK4 book generally avoids this.

Understood - but in this case I was looking at what macros do when developing callbacks - there is no macro setup for the forall() function as implemented in gintro.

Cdecl is used internally in the gintro macros, when a proc is called directly from GTK, as for the callback functions. The applyCSS() function looks more like an ordinary function called from Nim code, not like a callback, so cdecl may be not needed.

forall() is a wrapper around gtk_container_forall() and it takes a callback that must be called from C. The Nim compiler will not compile this code if applyCSS() is not marked as cdecl.

For the container test, may something like "if widget of gtk4.Container" work?

The problem seems to be that forall() has a low-level interface, and as you can see I'm constructing a widget from the pointer (as you've said, not the best). I don't know what the type of widgetPtr is when this is called, so I will probably be constructing the wrong type in gintro. According to tthe docs gtk_container_forall() also iterates over "internal" GTK widgets, where widgetPtr may not even be a widget I've created (but in this case it should still have a style applied).

Maybe you can use as parameter type for applyCSS() Container instead of Widget?

This was my initial approach, but because it's called recursively, it doesn't work.

I was not able to see the whole Python code, I clicked on that git repository, but have not managed to navigate. So I don't know how large and complex the whole GUI part is. Is it GTK4 or still old GTK3?

Exact function is here, and is only used in this file. https://git.sr.ht/~martijnbraam/numberstation/tree/master/item/numberstation/window.py#L82
The only calls to that function are in the same file, but it's the recursive call that is problematic.
It is still old GTK3.
Also my repository (which is a rough line-for-line translation) is here https://git.sr.ht/~scott__/numberstation-nim/tree/master/item/src/window.nim

Also note, that the fate of gintro is unclear.

That's a shame to hear - although I did run into a few challenges (and added a few hacks), porting this application was possible and the resulting nim-generated binary is faster to load than it's original, which is important as I'm running it on my pinephone.

... so I am considering removing it from github end of this years.

Even if the code is unmantained, I think there's still a benefit to leaving it on github (with a message about it being unmaintained), because it's still a useful reference for people trying to roll their own.

@StefanSalewski
Copy link
Owner

OK, now I understand: The ForEach() should have a callback proc as parameter, which is called from GTK directly. In that case we have to create a Nim macro, similar as our mconnect or iddleAdd macros. Unfortunately that is always some work, and there is always no help from the Nim core devs or someone other. Well I may try it in winter, but I do not intent to spent a few of the remaining summer days for that.

Have you considered using https://docs.gtk.org/gtk3/method.Container.get_children.html instead? I am not sure if that works, sometimes creating Nim seqs from glists works.

My impression is, that you are not the original author of the Python version. So creating a Nim version is some form of stealing, which I would not like to support? Is there some form of agreement from the original author available? Unfortunately, we had some people from Russia in the Nim community -- it is a bit difficult to ensure that gintro users do not support Putins war. When we can solve all these problems, then it may be the best strategy to first port the python code to GTK4, and then port to Nim.

For the generall gintro fate: After all these years (more than 1600 hours work from my side), we have nearly no users -- GTK generally has as users mostly plain consumer types, so no one is creating new GTK software at all. And for Nim many other GUI libs exists. Gobject-introspection is much more difficult than advertised by the GTK team. One of my ideas was to hire some Rust and GTK experts to create Nim bindings based on the Rust ones, as the Rust ones seems to be the best. I was willing to support that with up to 1000 Dollar or Euro, so when a few dozen people would support it with a similar amount of money, that would have been an option. But I think there is no actual hope for this idea.

@scott-asdfasdf
Copy link
Author

In that case we have to create a Nim macro, similar as our mconnect or iddleAdd macros.

I might be interested in having a go, since I assume the code I've already got will be similar to what is required? I'm still not sure the best way to do the introspection though. Also, this is my first try of Nim, so my knowledge of the language is still fairly limited.

Have you considered using https://docs.gtk.org/gtk3/method.Container.get_children.html instead? I am not sure if that works, sometimes creating Nim seqs from glists works.

There may still be some issues with doing it this way.

  • forall() applies to "internal" children as I mentioned above, getChildren() does not.
  • Is it safe to call getChildren() on widgets that are not containers? If so this might work, otherwise I'm still stuck needing to know what the type is.

... So creating a Nim version is some form of stealing, which I would not like to support?

I am not the original author of the code, and I do not have an agreement from the author - but I feel like what am I doing is absolutely not stealing, for the following reasons:

  • Firstly, this is just an exercise in learning Nim for me - there was no particular intention to release the app and the code I am writing is not of releasable quality. The choice of this program is because it's something that I use everyday, and since the pinephone is very slow, it was a good chance to see if using Nim made a substantial difference in this case (which it does).
  • The original code is licensed under LGPL, which means that it is free to edit and change, even without attribution. In fact, if I'm doing anything wrong, it's that I haven't removed the original author's name from my repository and he may not want to be associated with my code.
  • The original author is part of the postmarket OS team, and I don't think that community would have any issues with others modifying and experimenting with their code.
  • If I were to advertise this code back to that community, I would make sure to have correct attribution (that my code is a derivative work of their code, etc). Really my repo is mainly a personal backup right now.

... it may be the best strategy to first port the python code to GTK4, and then port to Nim.

OK - although if I was writing this in C I would use the macro IS_CONTAINER(). Also porting this to C first (or another language) does reduce the benefits of writing in Nim (that it's a good compromise between slower python, and faster, but more error prone C).

For the generall gintro fate ..

I can see that a lot of work went into this library, so that is a shame - but I get that it's a huge effort on your part to maintain.

And for Nim many other GUI libs exists

GTK has had a lot of effort go into convergent apps recently, so it's currently good choice (one of a few options) for developing for the pinephone.

Thanks for your help so far.

@StefanSalewski
Copy link
Owner

I might be interested in having a go, since

Yes, it should be not too difficult. I generally grep one of the existing gintro macros, and just modify it. The mconnect() macro is a bit complicated, and ugly, as it still works mostly on strings, and not on AST. The other smaller macros like iddleAdd() are much simpler, one can start with such one. The cdecl vs Nim calling convention generates always some overhead for the gintro macros. There were rumors that cdecl and Nim calling convention may join for Nim 2.0, that may simplify a lot. For your app, one more problem is, that libhandy is used, which is mostly untested, and I think for GTK4 it does not exist and libadwaita is used. I have never used that one before.
Creating the foreach macro should be not that much work, maybe only one day for me. But all the testing is work too, maybe I would have to create a tiny libhandy app to test it. Maybe I can try next weekend.

When you are new to Nim, have you read my book or the new one from Mr. Rumpf already?

@StefanSalewski
Copy link
Owner

I was going to start creating the foreach macro. I think the idleAdd() is not a bad starting point, but it will need some serious changes. Unfortunately all of our macros are mostly based on string operations, not on AST manipulation. For the mconnect macro we learned that this can give some serious problems, which may vanish for pure AST manipulation. But getting comfortable in AST manipulation is not that easy, rewriting the mconnect in pure AST manipulation may took me 4 or six weeks fulltime. Well, others, like Mr Rumpf may be able to do it much faster, but I think you will not be able to get him doing that. Same for Mr Doehring or Disruptec and the other macro experts.

So I will try again a macro based on string manipulation for now.

I just learned that https://docs.gtk.org/gtk3/method.Container.forall.html is gone in GTK4, see https://blog.gtk.org/2017/04/25/widget-hierarchies-in-gtk-4-0/. So all the work is actually useless, as expected already.

I will tell you when I have a first prototype, maybe in about one week.

@StefanSalewski
Copy link
Owner

OK, I have the first skeleton foreach() macro already working, after less than 3 hours. IdleAdd() was indeed a fine starting point, and the macro is basically very simple. I hope I can ship it tomorrow.

@StefanSalewski
Copy link
Owner

Well, I assume you have already retired, but I have just added a first try of a foreach macro. Unfortunately it works currently only with ref parameters. I just tried to make it work with value types like strings, but failed for now. The macro is ugly, but should work for you, and you get the basic idea. When you have read the book of Mr. Rumpf, you may improve it yourself, maybe rewrite to do only AST manipulation. I tested it with this program:

You can update gintro with

nimble uninstall gintro
nimble install gintro@#head
import gintro/[gtk, glib, gobject, gio]
import std/macros
import std/sequtils
from std/strutils import startsWith, replace, `%`

proc applyCss(w: Widget; provider: CssProvider) =
  discard
  
#proc setLabelText(w: Widget; provider: CssProvider) =
proc setLabelText(w: Widget; s: ref string) =
  echo "setLabelText"
  if w of gtk.Label:
    echo "w is gtk.Label"
    gtk.Label(w).setText(s[])
    
proc buttonClicked (button: Button) =
  button.label = utf8Strreverse(button.label, -1)

proc appActivate (app: Application) =
  let window = newApplicationWindow(app)
  window.title = "GNOME Button"
  window.defaultSize = (250, 50)
  let vbox = newBox(Orientation.vertical, 10)
  let button = newButton("Click Me")
  let label1 = newLabel("Label 1")
  vbox.add(button)
  vbox.add(label1)
  button.connect("clicked",  buttonClicked)
  window.add(vbox)
  var prov: CssProvider
  vbox.foreach(applyCss, prov) # dummy test
  var str: ref string
  new(str)
  str[] = "Hello"
  vbox.foreach(setLabelText, str)
  window.showAll

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

main()

@StefanSalewski
Copy link
Owner

And for your source file:


Instead of

result.addName = cast[Entry](builder.getObject("add_name"))

something like

result.addName = builder.getEntry("add_name")

should work since a few years now.

All the callbacks like

proc on_add_entry_clicked(widget: Widget, userData: pointer) {.cdecl,exportc,dynlib.} =
  let self: NumberstationWindow = cast[NumberstationWindow](userData)
  self.mainstack.setVisibleChildName("add")

should finally collapse to something like

proc onAddEntryClicked(widget: Widget; self: NumberstationWindow) =
  self.mainstack.setVisibleChildName("add")

And all the cstring() calls are useless of course:

dialog.setLicense(cstring(text))

Older Nim versions passed Nim strings fine. When Mr. Rumpf thinks we should use the ugly cstring() now
 -- well, them we can just use Rust with its ugly some("MyText").

@StefanSalewski
Copy link
Owner

OK, this macro version seems to work with strings:

macro foreach*(container: gtk.Container; p: untyped; arg: typed): untyped =
  var foreachID {.compileTime, global.}: int
  inc(foreachID)
  var ats = $getTypeInst(arg).toStrLit
  let procName = "foreach_" & $foreachID
  let procNameCdecl = "foreach_cdecl_" & $foreachID
  var r1s = """
proc $1(c: ptr gtk.Widget00; p: pointer) {.cdecl.} =
  when $3 is ref:
    let a = cast[$3](p)
  else:
    let a = cast[ref $3](p)
  let h: pointer = g_object_get_qdata(c, Quark)
  assert(h != nil)
  let cn: gtk.Container = cast[gtk.Container](h)
  when $3 is ref:
    $2(cn, a)
  else:
    $2(cn, a[])
""" % [$procNameCdecl, $p, ats]
 
  let r2s ="""
proc $1(container: gtk.Container; a: $2) =
  when a is ref:
    GC_ref(a)
    gtk_container_foreach(cast[ptr gtk.Container00](container.impl), $3, cast[pointer](a))
  else:
    var ar: ref $2
    new(ar)
    ar[] = a
    GC_ref(ar)
    gtk_container_foreach(cast[ptr gtk.Container00](container.impl), $3, cast[pointer](ar))
$1($5, $4)
""" % [$procName, ats, $procNameCdecl, $arg, $container]
  echo r1s
  echo r2s
  result = parseStmt(r1s & r2s)

I will ship it to GitHub soon.

@scott-asdfasdf
Copy link
Author

I've had a very busy month, so I haven't had a chance to come back to this until now, sorry.

Just to reply to a few of the previous comments:

When you are new to Nim, have you read my book or the new one from Mr. Rumpf already?

Not in full, but I have read sections from both when trying to work out how to do specific things.

I just learned that https://docs.gtk.org/gtk3/method.Container.forall.html is gone in GTK4

OK - I changed my code to use foreach instead and it seems to have the same effect, so I'll just use foreach instead.

something like
result.addName = builder.getEntry("add_name")

This mostly works well thanks - although there don't seem to be replacements for these lines (I guess these are uncommon cases though):

let windowObj = builder.getObject("main_window")
result.window = cast[handy.ApplicationWindow](windowObj)

result.addLengthAdj = cast[Adjustment](builder.getObject("add_length_adj"))

[Callbacks] should finally collapse to something like ...

No I don't think this will work. These callbacks are called from C, but instead of being setup in code, the callbacks are set in the glade file using the handler attribute (note the glade file is unmodified from the original project). I believe the callbacks are connected using the builder.connectSignals(cast[pointer](result)) call. It's amazing that this mostly just works, but it does - it requires compiling with switch("passL", "-rdynamic"). I have no idea how this works in python.

OK, this macro version seems to work with strings:

I tried with the new version, but I have a few issues because of my use case (which maybe just isn't supported by gintro).
This was my new attempt (looks much cleaner):

proc applyCSS(widget: Widget, provider: CSSProvider) =
  widget.getStyleContext().addProvider(provider, STYLE_PROVIDER_PRIORITY_APPLICATION)

  if isContainer(cast[ptr Widget00](widget.impl)) > 0:
    var container: Container = cast[Container](widget)
    foreach(container, applyCSS, provider)
  • Recursive use of foreach still requires knowing if the widget is a container or not - so I still need an ugly hack to do that.
  • The new version wouldn't compile with nim 1.66 (don't know what version you are using?). It complained about indenting and I had to remove one level of spaces in the macro definition, and also add an extra new line to the beginning of r2s. It did compile successfully after that.
  • The new version doesn't run in this setup, it throws an assertion (h == nil). I assume when gintro creates widgets it sets the user data for those widgets so the pointer can be recovered later? Unfortunately when the widgets are created through the builder (glade) it seems like the user data is missing.

Thanks for your efforts - it's been an interesting learning experience.

@scott-asdfasdf
Copy link
Author

Actually, one last follow up - I've realised why the assertion keeps happening. It's not because of the builder, it's because a button is a container (which I didn't realise) and gtk_button_set_label() adds a GtkLabel to the button (which bypasses the bindings completely for that constructor).

If I modify my applyCSS() to deliberately construct all children, it works:

proc applyCSS(widget: Widget, provider: CSSProvider) =

  widget.getStyleContext().addProvider(provider, STYLE_PROVIDER_PRIORITY_APPLICATION)

  if isContainer(cast[ptr Widget00](widget.impl)) > 0:
    var container: Container = cast[Container](widget)
    discard container.getChildren()
    foreach(container, applyCSS, provider)

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

No branches or pull requests

2 participants