bytedeco / javacpp

The missing bridge between Java and native C++
Other
4.43k stars 576 forks source link

Class instance state is reset between calls #756

Closed damien-vdb closed 1 month ago

damien-vdb commented 2 months ago

I am trying to bind my Java application to a legacy C++ library with JavaCPP. I am using the gradle plugin with the Parser, and for the simpler definitions it worked almost out of the box :+1:

I have currently 2 issues:

  1. The state between calls on an object instance is reset
  2. Complex mappings with the Parser

Here is the exported interface I'm calling:

#ifndef _ICAR_H
#define _ICAR_H

#if _GNUC__ >=4
    #define DLL_CAR_PUBLIC __attribute__ ((visibility ("default")))
    #define DLL_CAR_LOCAL __attribute__ ((visibility ("hidden")))
#else
    #define DLL_CAR_PUBLIC
    #define DLL_CAR_LOCAL
#endif

class ICar
{
public:
    typedef unsigned long Identifier;

    virtual ~ICar() {};

    virtual bool methodA(unsigned long nb, Identifier* ids) = 0;
    virtual bool methodB(short mode, unsigned long* nb, Identifier*& ids) = 0;
    virtual bool methodC(std::map<Identifier, std::map<Identifier, double> >& map) = 0;

};

extern "C" DLL_CAR_PUBLIC ICar * create();
extern "C" DLL_CAR_PUBLIC void destroy(ICar* car);

#endif

I managed to find the relevant source code, here is the subclass:

#ifndef _CCAR_H
#define _CCAR_H

class CCar : public ICar
{
public:
    typedef AppCar::Identifier Identifier;

    CCar() {};
    virtual ~CCar();

    virtual bool methodA(unsigned long nb, Identifier* ids) override;

    virtual bool methodB(short mode, unsigned long* nb, Identifier*& ids) override;

    virtual bool methodC(std::map<Identifier, std::map<Identifier, double> >& map) override;

protected:
    AppCar m_appCar;
};

#endif

The implementation:

CCar::CCar()
: m_appCar() {}

CCar::~CCar() {}

bool CCar::methodA(unsigned long nb, Identifier* ids) {

    return m_appCar.methodA(nb, ids);
}

bool CCar::methodB(short mode, unsigned long* nb, Identifier*& ids) {

    return m_appCar.methodB(mode, nb, ids);
}

# Same for methodC

And the delegate where all the logic is done:

AppCar::AppCar()
: m_methodACalled(false), 
  m_methodBCalled(false) {
    reset() # Is this weird ?
}

AppCar::~AppCar() {}

bool AppCar::reset() {
  m_methodACalled = false;
  m_methodBCalled = false;
  return true;
}
bool AppCar::methodA(unsigned long nb, Identifier* ids) {
    m_methodACalled = true;
    return true;
}
bool AppCar::methodB(short mode, unsigned long* nb, Identifier*& ids) {
    if (!m_methodACalled) {
        return false;
    }
    m_methodBCalled = true;
    return true;
}
bool AppCar::methodC(std::map<Identifier, std::map<Identifier, double> >& map) {
    if (!m_methodBCalled) {
        return false;
    }
    return true;
}

My corresponding configuration:

@Properties(
        value = @Platform(include = "ICar.h", link = "Cars"),
        target = "com.example.Car"
)
class CarConfig implements InfoMapper {
    public void map(InfoMap infoMap) {
        infoMap.put(new Info("DLL_CAR_PUBLIC", "DLL_CAR_LOCAL").cppTypes().annotations())

                // Issue 2: Can't manage to map Identifier*& parameters. It should map to @ByPtrRef CLongPointer ?
                // Otherwise I end up with "cannot bind non-const lvalue reference of type 'ICar::Identifier*&' ... 

                // Also, after writing the mapping for the map of Identifiers, the Parser starts to fail, so I tried to
                // force the simple mappings back:
                .put(new Info("ICar::Identifier")
                        .valueTypes("@Cast(\"ICar::Identifier\") CLongPointer")
                        .pointerTypes("@Cast(\"ICar::Identifier*\") CLongPointer")
                        .define()
                )
                .put(new Info("std::map<ICar::Identifier,std::map<ICar::Identifier,double> >").pointerTypes("DoublesPerIdPerId").define())
                .put(new Info("Car::ICar").purify(true))
                // Lines below are not referenced in the example
                .put(new Info("std::map<int,std::vector<ICar::Identifier>").pointerTypes("IdsPerInt").define())
                .put(new Info("std::vector<ICar::Identifier>").pointerTypes("IdVector").define());
    }
}

And the test run:

public class CarDemo {

    public static void main(String[] args) {
        try(ICar car = Car.create()) {
            car.methodA(...);
            car.methodB(...); // Fails, as if methodA was never called
        }
    }
}

Issue 1: Even though I am using the same Car object, the boolean states seem to be reset between calls. I found out another C++ client successfully uses the Car like this:

std::unique_ptr<ICar, void (*)(ICar *)> car = std::unique_ptr<ICar, void (*)(ICar *)>(create(), destroy);

Should I use @UniquePtr somewhere ?

Issue 2: Even with the wiki I struggle with InfoMap declarations. Am I on the right track ?

Thank you

saudet commented 2 months ago

We don't need to use UniquePtr if it's not in the API, no. It looks like you want to use the methods from ICar with a Car object, so we might need to use Info.virtualize() for that to work.

damien-vdb commented 2 months ago

Thank you for the quick feedback.

I'm sorry that the example might lead to confusion because of the naming: class Car { ICar create(); void destroy(ICar car) } is the Javacpp generated class englobing ICar. I don't want to override anything, I just want to instanciate an ICar, through the create that was provided (which internally only does a new CCar()..).

while for classes containing virtual methods that we would like to override in Java, we can use Info.virtualize

Therefore I don't think I need to virtualize() anything.

saudet commented 2 months ago

Well, if create() actually returns a Car instance, Java doesn't know that. I'm pretty sure we'll need to map dynamic_cast<Car>() somewhere to cast it into a Car so that we can use it from Java as a Car.

@HGuillemet I think you added something for that use case, right?

damien-vdb commented 1 month ago

I finally found the solution for my first issue: instead of .skip() some method declarations in my InfoMap, I commented out the .h declarations, which is apparently not a good way.

Hope it helps some beginners like myself.

For the second issue, I'll look a bit more to the existing java-presets for inspiration. I'm closing the issue.

HGuillemet commented 1 month ago

Well, if create() actually returns a Car instance, Java doesn't know that. I'm pretty sure we'll need to map dynamic_cast<Car>() somewhere to cast it into a Car so that we can use it from Java as a Car.

@HGuillemet I think you added something for that use case, right?

if create returns a pointer, says p to an object of type ICar, Java will get an object of class ICar, say c, that will hold p. If we call c.methodA() from Java, this will map to ((ICar *)p)->methodA(), which should call the implementation from CCar. So that should work without adding anything. Unless there are some multiple or virtual inheritance involved that @damien-vdb didn't talk about. Are there ?