Clipper2 icon indicating copy to clipboard operation
Clipper2 copied to clipboard

Export classes and functions from library for Windows & Linux

Open damiandixon opened this issue 3 years ago • 10 comments

The DLL exporting of classes and methods on Windows requires them to be exported using:

#ifndef CLIPPER2_DLL
#  if defined(_WIN32)
#    ifdef CLIPPER2_DLL_EXPORT
#      define CLIPPER2_DLL __declspec(dllexport)
#    else
#      define CLIPPER2_DLL __declspec(dllimport)
#    endif
#  else
#    if __GNUC__ >= 4
#      define CLIPPER2_DLL __attribute__((visibility("default")))
#    else
#      define CLIPPER2_DLL
#    endif
#  endif
#endif

The above should be in a header file on its own and included as needed.

With classes defined:

class CLIPPER2_DLL Clipper2Class ....

When building the define CLIPPER2_DLL_EXPORT is declared. When linking to a library the define is not set.

On Linux the classes and methods will also be exported correctly when the option -fvisibility=hidden is used when creating a shared library.

Also see #169

damiandixon avatar Sep 30 '22 07:09 damiandixon

Following up on my question here... I'm still not persuaded that exporting classes is better that exporting well designed functions. For example ISTM that the following function would provide all the functionality of directly accessing a Clipper64 object:

  static const unsigned CLIP_FLAG_REVERSE_SOLUTION = 1;
  static const unsigned CLIP_FLAG_REMOVE_COLLINEAR = 2;

  inline bool BooleanOp(ClipType cliptype, FillRule fillrule,
    const Paths64& subjects, const Paths64& open_subjects, const Paths64& clips,
    Paths64& closed_solutions, Paths64& open_solutions, unsigned clip_flags = 0)
  {
    Clipper64 clipper;
    clipper.AddSubject(subjects);
    clipper.AddOpenSubject(open_subjects);
    clipper.AddClip(clips);
    clipper.PreserveCollinear = !(clip_flags & 2);
    clipper.ReverseSolution = (clip_flags & 1);
    return clipper.Execute(cliptype, fillrule, closed_solutions, open_solutions);
  }

Of course there would need to be a similar function that returned PolyTree64, and the same for ClipperD.

AngusJohnson avatar Oct 04 '22 00:10 AngusJohnson

May I add my two cents, please?

I don't know how/is the version compatibility check is done in Windows world nowadays and was it ever implemented in MS C++ compiler system-wide. so to say (last time I coded something for Windows in the beginning of zeroes).

Nevertheless, I don't see a problem to implement such a check: there is a number of ways to call a function before Main in C++. So that function defined inside clipper2.hpp and only compiled during builds of 'importing' applications (#ifdef-ed), so it calls a getVersion function from the dll and aborts the app execution on version mismatch. For the unix world this is done by .so versioning, thus no need for runtime check.

As for defining wrappers - it would only increase code maintenance difficulty: every time you change any method declaration to be exported you have to change the interface function. So, this is really unnecessary level of indirection and an additional source of possible bugs.

sergey-239 avatar Oct 04 '22 01:10 sergey-239

As for defining wrappers - it would only increase code maintenance difficulty

Well I'm certainly motivated to avoid code maintenance as much as possible. And part of that is keeping the code as simple as possible. Anyhow, I'm not against exporting classes in principle, I'm just hoping for a dll/so solution that works well on all platforms.

AngusJohnson avatar Oct 04 '22 02:10 AngusJohnson

When Clipper has a permissive license and compiles to a reasonably small binary, what exactly is the use case for a Clipper.dll on Windows?

reunanen avatar Oct 04 '22 14:10 reunanen

When Clipper has a permissive license and compiles to a reasonably small binary, what exactly is the use case for a Clipper.dll on Windows?

The C implementation is the fastest. You could convert this into a DLL. Delphi people can then just use these dll and take advantage of the speed.

rs0xFFFF avatar Oct 04 '22 15:10 rs0xFFFF

But Delphi surely can't import C++ classes? So for that use case, there should be a plain C API, right?

reunanen avatar Oct 04 '22 17:10 reunanen

Yep!

rs0xFFFF avatar Oct 04 '22 17:10 rs0xFFFF

But Delphi surely can't import C++ classes? So for that use case, there should be a plain C API, right?

And why I'm still (very slightly) leaning toward function wrappers instead of exposing classes.

AngusJohnson avatar Oct 04 '22 20:10 AngusJohnson

But Delphi surely can't import C++ classes? So for that use case, there should be a plain C API, right?

And why I'm still (very slightly) leaning toward function wrappers instead of exposing classes.

Yes, this changes the picture. Btw, the same dll may expose C and C++ symbols at the same time, so basically, Delphi library becomes just a binding...

sergey-239 avatar Oct 04 '22 22:10 sergey-239

http://rvelthuis.de/articles/articles-cppobjs.html http://rvelthuis.de/articles/articles-dlls.html http://rvelthuis.de/articles/articles-cobjs.html http://rvelthuis.de/articles/articles-convert.html

rs0xFFFF avatar Oct 04 '22 23:10 rs0xFFFF

I've just added a new clipper.export.h file to create DLL/so files and a sample delphi app to test. I see this file as a proof of concept rather than a definitive response to the above discussion.

AngusJohnson avatar Oct 25 '22 22:10 AngusJohnson

Seems to be a 32 bit problem. DLL_32bit

rs0xFFFF avatar Oct 27 '22 08:10 rs0xFFFF

I can't duplicate this. Are you using the latest commit?

procedure DoRandomStressTest(maxCnt: integer);
var
  i: integer;
  sub, clp: TPathsD;
  csub_local, cclp_local: CPathsD;
  csol_extern, csolo_extern: CPathsD;
begin
  csolo_extern := nil;
  SetLength(sub, 1);
  SetLength(clp, 1);

  for i := 1 to maxCnt do
  begin
    sub[0] := MakeRandomPathD(400, 400, 50);
    clp[0] := MakeRandomPathD(400, 400, 50);
    csub_local := TPathsDToCPathsD(sub);
    cclp_local := TPathsDToCPathsD(clp);

    BooleanOpD(Uint8(TClipType.ctIntersection),
      Uint8(TFillRule.frNonZero),
      csub_local, nil, cclp_local,
      csol_extern, csolo_extern);

    DisposeLocalCPathsD(csub_local);
    DisposeLocalCPathsD(cclp_local);
    DisposeExportedCPathsD(csol_extern);
    DisposeExportedCPathsD(csolo_extern);
    if i mod 100 = 0 then Write('.');    
  end;
  WriteLn(#10'Passed!');
end;

var
  s: string;
begin
  Randomize;
  DoRandomStressTest(10000);
  ReadLn(s);
end;

AngusJohnson avatar Oct 27 '22 08:10 AngusJohnson

Yes, I do Clipper2_32.zip

rs0xFFFF avatar Oct 27 '22 09:10 rs0xFFFF

Seems to be a 32 bit problem.

OK, I missed that bit.

Edit: cdecl; I'll upload the full fix shortly.

AngusJohnson avatar Oct 27 '22 09:10 AngusJohnson

Fix just uploaded.

AngusJohnson avatar Oct 27 '22 12:10 AngusJohnson

Hi Angus, thanks for the correction. Does the benchmark mean (4000 edges) that the C implementation is 6x faster than Delphi? Bad time for Delphi? :-( benchmark

rs0xFFFF avatar Oct 27 '22 15:10 rs0xFFFF

Hi Angus, thanks for the correction. Does the benchmark mean (4000 edges) that the C implementation is 6x faster than Delphi? Bad time for Delphi? :-(

Firstly, the performance test I've coded in the sample apps intentionally creates multiple intersections at the same locations as a kind of stress test of the clipping algorithm. And this also significantly slows clipping. This is important because it's not just the number of edges that affects timing, but the size of the random polygons too. Larger polygons will have fewer co-located intersections so clipping will be faster. Perhaps surprisingly ClipperD performance will usually appear to be much faster than Clipper64, but this is simply because ClipperD obects by default scale their paths *100. Consequently their polygons are much bigger (during clipping) and will have fewer co-located intersections. IOW, make sure you're comparing Clipper64 with Clipper64 (or comparing Clipper64 with ClipperD without scaling) and make sure the random polygons are generated into the same regions.

AngusJohnson avatar Oct 27 '22 21:10 AngusJohnson

Okay, understood, of course I have to take that into account.

rs0xFFFF avatar Oct 27 '22 23:10 rs0xFFFF