romgrk / node-gtk

GTK+ bindings for NodeJS (via GObject introspection)
MIT License
494 stars 41 forks source link

Implement register class #268

Closed romgrk closed 3 years ago

romgrk commented 3 years ago

Currently, it is possible to extends GObject on the javascript side, but as far as GIR is concerned, those are of the same type as their parent. Allowing the creation of new gobject types integrates those subclasses fully with the type system. It will also allow us to implement virtual functions.

class CoolButton extends Gtk.Widget {
  static GTypeName = 'NodeGtkCoolButton'
  snapshot() {
    console.log('implementation of virtual function')
  }
}
gi.registerClass(CoolButton)

Tasks:

romgrk commented 3 years ago

We can now create custom GTK widgets and have them available for inspection in the GTK inspector :confetti_ball:

Screenshot from 2021-03-11 05-51-56

romgrk commented 3 years ago

We can also paint (with the new GTK4 API) and see our render nodes in the inspector recorder tab.

Screenshot from 2021-03-11 06-42-23

romgrk commented 3 years ago

macOS failing again :( @wotzlaff if you have some time to run the failing test (tests/register-class.js) and see what comes out of it it would be great. The diff between the last passing tests and now is basically this.

wotzlaff commented 3 years ago

What we get is

zsh: illegal hardware instruction  node tests/register-class.js

which sounds pretty bad.

Running lldb might pin down the issue a bit, but I am not able to get any relevant information from the output:

nico@Nicos-iMac node-gtk % lldb -- node tests/register-class.js
(lldb) target create "node"
Current executable set to 'node' (x86_64).
(lldb) settings set -- target.run-args  "tests/register-class.js"
(lldb) r
Process 3886 launched: '/usr/local/bin/node' (x86_64)
node_gtk.node was compiled with optimization - stepping may behave oddly; variables may not be available.
Process 3886 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    frame #0: 0x000000010553acf9 node_gtk.node`GNodeJS::ObjectClass::RegisterVFunc(Nan::FunctionCallbackInfo<v8::Value> const&) [inlined] GNodeJS::ObjectClass::FindVFuncInfo(implementor_gtype=<unavailable>, info=0x0000000105a3bd90, fieldInfoOut=<unavailable>) at gobject.cc:0:22 [opt]
   1    
   2    #include <string.h>
   3    
   4    #include "boxed.h"
   5    #include "callback.h"
   6    #include "closure.h"
   7    #include "debug.h"
Target 0: (node) stopped.

Maybe you have an idea what's going on here...

BTW: It says that node-gtk is compiled with optimization but I do not know how to turn that off. If you have a hint about that we might get some more information here.

romgrk commented 3 years ago

Oh, that is strange. Do you get the same behavior if you test it at 6e93afc?

And for debug builds, run first npx node-pre-gyp configure --debug, then npx node-pre-gyp build (or npm run build).

wotzlaff commented 3 years ago

The test is passing at 6e93afc.

Thanks for the hint about the debug build. I though npm run configure and npm run build was enough (because the commands are basically the same). Anyway, using the debug build we can pin it down to

Process 2117 launched: '/usr/local/bin/node' (x86_64)
Process 2117 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
    frame #0: 0x000000010554bc6b node_gtk.node`GNodeJS::BaseInfo::operator=(this=0x00007ffeefbfe170, info=0x0000000104871540) at gi.h:67:9
   64   
   65       inline GIBaseInfo& operator= (GIBaseInfo *info) {
   66           this->clear();
-> 67           _info = info;
   68       }
   69   
   70       inline GIBaseInfo* operator* () {
Target 0: (node) stopped.

But actually I still have no idea what this means.

Edit:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x000000010561cc6b node_gtk.node`GNodeJS::BaseInfo::operator=(this=0x00007ffeefbfe170, info=0x00000001058fd940) at gi.h:67:9
    frame #1: 0x000000010561a35d node_gtk.node`GNodeJS::ObjectClass::FindVFuncInfo(implementor_gtype=4380374736, info=0x0000000106a2ad90, name="focus", vtable=0x00007ffeefbfe268, fieldInfoOut=0x00007ffeefbfe260) at gobject.cc:789:20
    frame #2: 0x0000000105619f83 node_gtk.node`GNodeJS::ObjectClass::RegisterVFunc(info=0x00007ffeefbfe740) at gobject.cc:864:10
    frame #3: 0x0000000105616fb5 node_gtk.node`RegisterVFunc(info=0x00007ffeefbfe740) at gi.cc:355:5
    frame #4: 0x000000010560b749 node_gtk.node`Nan::imp::FunctionCallbackWrapper(info=0x00007ffeefbfe820) at nan_callbacks_12_inl.h:176:3
romgrk commented 3 years ago

Can you try showing the assembly instructions? Not sure how to do it in lldb, this might help.

With gdb I get something like this after pressing Ctrl-x A, Ctrl-x 2: Screenshot from 2021-03-15 09-26-33

wotzlaff commented 3 years ago

Does this help?:

(lldb) disassemble --frame
node_gtk.node`GNodeJS::BaseInfo::operator=:
    0x10554bc40 <+0>:  pushq  %rbp
    0x10554bc41 <+1>:  movq   %rsp, %rbp
    0x10554bc44 <+4>:  subq   $0x20, %rsp
    0x10554bc48 <+8>:  movq   %rdi, -0x8(%rbp)
    0x10554bc4c <+12>: movq   %rsi, -0x10(%rbp)
    0x10554bc50 <+16>: movq   -0x8(%rbp), %rax
    0x10554bc54 <+20>: movq   %rax, %rdi
    0x10554bc57 <+23>: movq   %rax, -0x18(%rbp)
    0x10554bc5b <+27>: callq  0x1055794f6               ; symbol stub for: GNodeJS::BaseInfo::clear()
    0x10554bc60 <+32>: movq   -0x10(%rbp), %rax
    0x10554bc64 <+36>: movq   -0x18(%rbp), %rcx
    0x10554bc68 <+40>: movq   %rax, (%rcx)
->  0x10554bc6b <+43>: ud2    
    0x10554bc6d <+45>: nopl   (%rax)
wotzlaff commented 3 years ago

Well done. It seems to work now.

romgrk commented 3 years ago

Cool ^^ Got my answer from here and here. Thanks a lot for the help.

wotzlaff commented 3 years ago

Maybe we should get rid of all compiler warnings somehow. At least this issue came up as warning, too.

romgrk commented 3 years ago

Yes we should really get rid of warnings -.- I'd be happy to enable -Werror eventually. I'll be rehauling the testing setup soon, I'll try to do the change then.

romgrk commented 3 years ago

Proper testing of vfuncs will need to wait for #269, so I'll merge this as it is.