go-qml / qml

QML support for the Go language
Other
1.96k stars 187 forks source link

Handle struct values (non-pointer) #67

Open quarnster opened 10 years ago

quarnster commented 10 years ago

Occurs in both v0 and v1:

package qml_test

import (
    "gopkg.in/qml.v1"
    "testing"
)

func init() {
    qml.SetupTesting()
}

func TestBlah(t *testing.T) {
    //qml.Init(nil)
    type Blah struct {
        A int
    }

    f := func() error {
        e := qml.NewEngine()
        defer e.Destroy()
        e.Context().SetVar("blah", Blah{})
        c, err := e.LoadString("blah.qml", `
import QtQuick 2.0
Item {
    Component.onCompleted: {
        blah.a = 10;
    }
}
`)
        if err != nil {
            return err
        }
        w := c.CreateWindow(nil)
        defer w.Destroy()
        return nil
    }
    err := f()
    if err != nil {
        t.Error(err)
    }
}

Yes I understand that it goes away if I do e.Context().SetVar("blah", &Blah{}), but that is besides the point.

Real use case is a go function that returns a small struct type by value. This value I then want to modify qml side to pass on this modified data to another go function.

niemeyer commented 10 years ago

If I understand what you're trying to achieve, this won't work, for the same reason this does not work:

http://play.golang.org/p/jMTwpE5bNo

Does that make sense?

If so, I'll change the topic of this issue as that error message is really bad and should be fixed.

quarnster commented 10 years ago

My point is that this works: http://play.golang.org/p/sbPAvbv6cf

niemeyer commented 10 years ago

The original point was about modifying the returned value, which does not work. But yeah, we can allocate a new value and copy it. Somewhat wasteful, but maybe it's worth to make such methods work.

niemeyer commented 10 years ago

Yeah, let's do that. Thanks for bringing this up.

quarnster commented 10 years ago

Making Go-QML "convert" return values into set-able types (if they weren't already) makes the QML script side behave more as the same code would work if it was written in Go, so IMO it's worth it.

There's already unavoidable overhead in every QML/Go function call boundary and any app where this boundary has a noticeable performance impact would do better in trying to get rid of the QML/Go boundary crossing in the first place.

The memory allocation could potentially be partially removed by using reflect.NewAt and keeping a small piece of memory for QML function stack, but I doubt the complexity of that is actually worth the effort :)

IIRC I also saw a panic-call when passing a "non-hashable" struct as a function argument or something when skimming the code. Would it be possible to use a similar approach there?

niemeyer commented 10 years ago

Yeah, the unhashable note is about the same issue. If we copy the value and get an address for it, both problems will go away.