AngusJohnson
Add Z-Component to Clipper DLL Exports, as well as Z-Fill Callback default
Added the Z-component and the Z-Fill callback. These are added when USINGZ preprocessor identifier is present Also added inflate for path (code was directly derived from code used for paths).
We depend on this Z-component for our functionality and rather not have a fork that diverges from your repository
This PR also has some contributions we'd like to make. Specifically for the Delphi interfaces to the C++. I added wrappers around the C++ raw pointer types for higher performance for the transmission of data.
You can see most of the code under DLL/Delphi_interface
- DLL/Delphi_interface/Clipper.DLL.Data.pas: a wrapper for the path and tree raw pointers passed to and from the DLL. Using this wrapper allows for faster in-stream memory processing rather than translating and converting to the Clipper Delphi datatypes.
- DLL/Delphi_interface/Clipper.DLL.pas: a DLL interface that takes type strong arguments, and allows allows for the use of the pointer wrappers provided by Clipper.DLL.Data for input and output.
- DLL/Delphi_interface/Clipper.DLL.Enums.pas: Strongly typed enums used by Clipper.DLL.pas.
- DLL/Delphi_interface/Clipper.DLL.inc: Controls z-component inclusion, double type coordinates and pointer wrappers
Consumers rarely need to include Clipper.DLL.Data or Clipper.DLL.Enums since the most commonly used types are aliased in Clipper.DLL.pas
All Clipper Delphi examples were converted to use these interfaces and can be found under DLL/Delphi_TestApps. I moved the test previously in DLL/Delphi_TestApp to a subfolder (DLL/Delphi_TestApp/Test_DLL).
- DLL/Delphi_interface/SVG/Clipper.DLL.SVG.pas and DLL/Delphi_interface/Utils/Clipper.DLL.Utils.pas: used by test apps to generate data and output. These are translations of @AngusJohnson's Delphi Clipper code but which now operates on the raw pointer wrappers
@AngusJohnson If this change is too large or if you'd rather not accept the Delphi pointer wrapper interfaces then I can make a smaller PR with just the z-component support on the DLL as well as the Z-fill callback export
Hi Jasper. Thank you for the PR, and from the little I've seen so far, it seems very nicely done (and well commented too). However, it'll probably take me a little while to work through and assimilate all your changes (possibly a week or two), which I'll want to do before I commit. So stay tuned 😁.
Thanks Jasper. I'm still here, but I'm currently caught up with other things.
Jasper, while I'm very slowly assimilating your proposed changes, and taking in to consideration your suggestions, I've tweaked the documentation in clipper.export.h further. And no doubt it'll need further tweaking.
/*
Boolean clipping:
cliptype: None=0, Intersection=1, Union=2, Difference=3, Xor=4
fillrule: EvenOdd=0, NonZero=1, Positive=2, Negative=3
Polygon offsetting (inflate/deflate):
jointype: Square=0, Bevel=1, Round=2, Miter=3
endtype: Polygon=0, Joined=1, Butt=2, Square=3, Round=4
The path structures used extensively in other parts of this library are all
based on std::vector classes. Since C++ classes can't be accessed by other
languages, these paths are converted here into very simple array data
structures that can be parsed by just about any programming language.
These 2D paths are defined by series of x and y coordinates together with an
optional user-defined 'z' value (see Z-values below). Hence, a vertex refers
to a single x and y coordinate (+/- a user-defined value). These values will
be either type int64_t or type double. Data structures have names that
indicate the array type by a suffixed '64' or a 'D'. For example, the data
structure CPath64 contains an array of int64_t values, whereas the data
structure CPathD contains an array of double. Where documentation omits
the type suffix (eg CPath), it is simply agnostic to the array's data type.
For conciseness, the following letters are used in the diagrams below:
N: Number of vertices in a given path
C: Count of structure's paths
A: Array size (as distinct from the size in memory)
CPath64 and CPathD:
These are arrays of consecutive vertices preceeded by a pair of values
containing the number of vertices (N) in the path, and a 0 value.
_______________________________________________________________
| counters | vertex1 | vertex2 | ... | vertexN |
| N, 0 | x1, y1, (z1) | x2, y2, (z2) | ... | xN, yN, (zN) |
---------------------------------------------------------------
CPaths64 and CPathsD:
These are also arrays containing any number of consecutive CPath
structures. Preceeding these consecutive paths, there is a pair of
values that contain the length of the array structure (A) and
the count of following CPath structures (C).
Memory allocation for CPaths64 = A * sizeof(int64_t)
Memory allocation for CPathsD = A * sizeof(double)
__________________________________________
| counters | path1 | path2 | ... | pathC |
| A, C | | | ... | |
------------------------------------------
CPolytree64 and CPolytreeD:
These structures consist of two values followed by a series of CPolyPath
structures. The first value indicates the total length of the array (A).
The second value indicates the number of following CPolyPath structures
that are the top level CPolyPath in the CPolytree (C). These CPolyPath
may, in turn, contain their own nested CPolyPath children that
collectively make a tree structure.
_________________________________________________________
| counters | CPolyPath1 | CPolyPath2 | ... | CPolyPathC |
| A, C | | | ... | |
---------------------------------------------------------
CPolyPath64 and CPolyPathD:
These array structures consist of a pair of counter values followed by a
series of polygon vertices and a series of nested CPolyPath children.
The first counter values indicates the number of vertices in the
polygon (N), and the second counter indicates the CPolyPath child count (C).
______________________________________________________________________________
|counts |vertex1 |vertex2 |...|vertexN |child1|child2|...|childC|
|N, C |x1, y1, (z1)| x2, y2, (z2)|...|xN, yN, (zN)| | |...| |
------------------------------------------------------------------------------
DisposeArray64 & DisposeArrayD:
All array structures are allocated in heap memory which will eventually
need to be released. However, since applications linking to these DLL
functions may use different memory managers, the only safe way to release
this memory is to use the exported DisposeArray functions.
(Optional) Z-Values:
Structures will only contain user-defined z-values when the USINGZ
pre-processor identifier is used. The library does not assign z-values
because this field is intended for users to assign custom values to vertices.
Z-values in input paths (subject and clip) will be copied to solution paths.
New vertices at path intersections will generate a callback event that allows
users to assign z-values at these new vertices. The user's callback function
must conform with the DLLZCallback definition and be registered with the
DLL via SetZCallback. To assist the user in assigning z-values, the library
passes in the callback function the new intersection point together with
the four vertices that define the two segments that are intersecting.
*/
I will update the PR with your text documentation changes in clipper.export.h.
EDIT: Done, see new commit 8755c6e, please let me know of any other changes as you find them
I will update the PR with your text documentation changes in clipper.export.h. EDIT: Done, see new commit
And I've just edited my post above 😱 with further tweaks to the documentation. I'm happy enough now not to tweak the documentation further and move on to testing 😁.
@AngusJohnson
I'm sure there will be other things to fine tune on this PR. I will wait for those and add the updated comments along with this. I don't mind doing this.
Alternatively, would you rather make a branch from main? I can edit the PR so it can be merge into another branch... that way it's easy for you to make changes as needed and merge into development or make a PR into main when ready. Many repositories require PRs be made into a separate branch and not directly to the main branch.
TestExportHeaders.cpp also needs updating ...
https://github.com/AngusJohnson/Clipper2/blob/c19da7257e7d3efdd02aede46d011fcf2f1bf4cc/CPP/Tests/TestExportHeaders.cpp#L12-L16
for (size_t i = 0; i < poly_len; ++i)
{
int64_t x = *v++, y = *v++;
#ifdef USINGZ
auto z = bit_cast<z_type>(*v++);
path.push_back(Point64(x, y, z));
#else
path.push_back(Point64(x, y));
#endif
}
https://github.com/AngusJohnson/Clipper2/blob/c19da7257e7d3efdd02aede46d011fcf2f1bf4cc/CPP/Tests/TestExportHeaders.cpp#L31-L41
static bool CreatePolyPathDFromCPolyPath(CPolyPathD& v, PolyPathD& owner)
{
size_t poly_len = static_cast<size_t>(*v++);
size_t child_count = static_cast<size_t>(*v++);
if (!poly_len) return false;
PathD path;
path.reserve(poly_len);
for (size_t i = 0; i < poly_len; ++i)
{
double x = *v++, y = *v++;
#ifdef USINGZ
auto z = bit_cast<z_type>(*v++);
path.push_back(PointD(x, y, z));
#else
path.push_back(PointD(x, y));
#endif
}
https://github.com/AngusJohnson/Clipper2/blob/c19da7257e7d3efdd02aede46d011fcf2f1bf4cc/CPP/Tests/TestExportHeaders.cpp#L47-L51
static bool BuildPolyTreeDFromCPolyTree(CPolyTreeD tree, PolyTreeD& result)
{
result.Clear();
double* v = tree;
int64_t array_len = static_cast<int64_t>(*v++);
size_t child_count = static_cast<size_t>(*v++);
https://github.com/AngusJohnson/Clipper2/blob/c19da7257e7d3efdd02aede46d011fcf2f1bf4cc/CPP/Tests/TestExportHeaders.cpp#L139-L142
for (int i = 0; i < 5; ++i)
{
EXPECT_TRUE(pp->Count() == 1);
pp = pp->Child(0);
}
https://github.com/AngusJohnson/Clipper2/blob/c19da7257e7d3efdd02aede46d011fcf2f1bf4cc/CPP/Tests/TestExportHeaders.cpp#L174-L177
for (int i = 0; i < 5; ++i)
{
EXPECT_TRUE(pp->Count() == 1);
pp = pp->Child(0);
}
Hi again Jasper. Yes, I am slowly progressing 😁, and I've attached a zip file containing quite a few changes. I will probably make a branch here soonish and upload into that, so this is a preview in case you want to see what I'm doing and make further suggestions / corrections.
You're probably aware that I'm not overly skilled in C++, so don't hold back if I seem to be doing something odd 😱. I would have liked to have made the PolyPath class generic as it would have simplified clipper.export.h, but unfortunately I can't see how to do that without creating havoc in clipper.engine.
@AngusJohnson I merged your code as well as the updated comments.
Since the exports need to be C-style there are few options to make this polymorphic. The only C-style way I know to make the calls polymorphic is to add a header in the data structures passed, the method then tests and calls the correct version for the data. Since double and int64 are the same size the size of the structures are always the same, so only the interpretation of the data changed. I don't mean the doubles can be treated as int64, I just mean from a data size view they cannot be distinguished. Alternative would have been an extra argument, but I'd rather take two calls like you have made (one with suffix of 64 and one with suffix of D) instead of passing an extra data. The way you have your exports is the best you can get with the datastructures as they are today, at least to the best of my understanding.
As a matter of opinion, I would have not included the double overloads, since internally you still operate only on the integral values. I would have expected this as the responsibility of the consumer.
WARNING: Rant on C++ follows:
As far as C++; I coded with it in the mid 90s, but recently learned the new stuff (11+) from Pluralsight (I did every single C++ course) and from YouTube. Pluralsight training was more purist approach and while a good foundation it tended to preach in absolutes, I found the The Cherno's C++ Videos more practical and balanced. When I tried to use Boost - which I believe is now close to 90% meta programming - I struggled to understand Template Meta Programming, but I found Bits of Q channel that explained it very well.
I am not a modern C++ fan, its a bloated bastard mix of 3 languages (C++, Preprocessor and one of many flavors of Template Metaprogramming). The more I mature as a dev the more I move away from OO, specifically virtual methods and inheritance, Template programming allows us to avoid inheritance and virtual methods and gain runtime speed, but I found it a cumbersome and error prone layered on language. So it gives us the ability to do these things, but sucks the joy out of coding
As a matter of opinion, I would have not included the double overloads, since internally you still operate only on the integral values. I would have expected this as the responsibility of the consumer.
I agree with you that the library really only needs to support integral values, and leave float conversions to consumers. And that is what I did in Clipper1. But I got so many requests and or questions as to why the library didn't support float values that I relented in Clipper2.
I am not a modern C++ fan, its a bloated bastard
I feel the same. And as you've mentioned the Boost library, I find its code almost unintelligible. C++ reminds me of my early days of programming. Back in the 1980s Ashton Tate brought out a DBase III+ compiler (the first version was Clipper 🤣 Summer '87) that was extremely popular because the language was so intuitively simple. But later versions of Clipper (up to 5.2), while much more powerful, destroyed the simplicity of the language. And this made reading other people's code and even maintaining my own code difficult. But, given the very large number of programming languages today, it's obvious that creating a really good programming language is extremely difficult. (And I shy away from describing what I mean by really good because that would be another essay.) Edit: And of course there are parallels between writing a really good programming language and writing a really good software library 😁.
I was wrong. There is a way to make it polymorphic! Your double structures use a double type to store integer values. So, it's easy to detect if the structure contains doubles. Any integer value stored in a double that is larger than zero (such as array size) can be bit-cast to int64 and if that is equal to or larger than 4,607,182,418,800,017,408 (3FF0 0000 0000 0000 hex) we know we have doubles. Since it's safe to assume arrays will never be that large, we have a detection system.
Looking at the structures, you can simply use the very first value, read it as int64 and then direct to the double or int64 branch based on this logic.
I use what I call Data Descriptors for polymorphic data traversal in external C-Style APIs. It describes the offset, size, stride, and indirection of the data that needs to be traversed. Doing this avoids the conversion and allows virtually any external data shape to be navigated by the API, without converting the data (expensive). It simply tells the API how to "walk" the data. I will see if I have time to write an article on this topic on my blog in the next week or so.
I was wrong. There is a way to make it polymorphic! Your double structures use a double type to store integer values. So, it's easy to detect if the structure contains doubles. Any integer value stored in a double that is larger than zero (such as array size) can be bit-cast to int64 and if that is equal to or larger than 4,607,182,418,800,017,408 (3FF0 0000 0000 0000 hex) we know we have doubles.
That's a very interesting observation 👍 but I haven't given any thought yet on how this could be utilised. (I'm still very tied up with other things.)
That's a very interesting observation 👍 but I haven't given any thought yet on how this could be utilised. (I'm still very tied up with other things.)
I would leave your int64\double APIs separate as is, because otherwise you have to take a void* argument. Let me know if you need any more changes on this item
Let me know if you need any more changes on this item
No. Please hold off for now. I'm hoping to upload my own changes in the next few days into a new branch. (And since I've never branched a GitHub repository before, I'm hoping I don't make a mess of things 🤞).
I've just finished a Delphi test app for the DLL (with and without USINGZ) and am about to start again on the C# apps. I'm aware that you put a lot of effort into your own Delphi test apps that use generics quite extensively. Thank you for that, but I'm disinclined to use generics in Delphi (in Clipper) since it limits compilers to D2009 and later. I have noticed, and am somewhat surprised, that quite a few programmers still seem to be using older compilers, though perhaps this is mostly for legacy apps.
No. Please hold off for now. I'm hoping to upload my own changes in the next few days into a new branch. (And since I've never branched a GitHub repository before, I'm hoping I don't make a mess of things 🤞).
I've just finished a Delphi test app for the DLL (with and without USINGZ) and am about to start again on the C# apps. I'm aware that you put a lot of effort into your own Delphi test apps that use generics quite extensively. Thank you for that, but I'm disinclined to use generics in Delphi (in Clipper) since it limits compilers to D2009 and later. I have noticed, and am somewhat surprised, that quite a few programmers still seem to be using older compilers, though perhaps this is mostly for legacy apps.
I understand. I believe only Clipper.DLL.Data has generics, if you want I can quickly convert that to a non-generic form, should not take very long, basically a find and replace, along with a version conditional for DataOfType<Q: Class>. Would you like me to take care of that?
Would you like me to take care of that?
Thanks for the offer but no. I've already redone the Delphi apps. (Two very simple apps, one with USINGZ and using callback.)
I noticed I added CLIPPER_USE_POINTER_WRAPPERS conditional define that can be turned off to eliminate those types that have generics. That define if not present falls back to using PInt64 and PDouble. I don't know how far you are in your changes, but a simple version check undefining that conditional in Clipper.DLL.inc for versions older than Delphi 2009 is probably the simplest fix. Unfortunately, some of the test apps use the new helper types
I've uploaded my changes into the export_usingz branch, and note that the bit_cast function in clipper.export.h fails to compile on all platforms except Windows. The sample DLL apps still need a bit of tidying, and probably would benefit with a few more samples. Nevertheless I hope to keep each sample fairly simple.
I've uploaded my changes into the
export_usingzbranch, and note that thebit_castfunction inclipper.export.hfails to compile on all platforms except Windows. The sample DLL apps still need a bit of tidying, and probably would benefit with a few more samples. Nevertheless I hope to keep each sample fairly simple.
I probably should have used reinterpret_cast which has been supported since C++ 11
#include <iostream>
#include <cstdint>
#include <cstring>
#include <cassert>
// Basically the same as bit_cast
int64_t doubleToInt64(double value) {
int64_t result;
std::memcpy(&result, &value, sizeof(value));
return result;
}
int main() {
for (int i = 1; i <= 10; ++i) {
double d = static_cast<double>(i);
int64_t int64Value = doubleToInt64(d); // bit casting (memory copied)
int64_t reinterpretedInt64 = *reinterpret_cast<int64_t*>(&d); // pointer reinterpreted
assert(int64Value==reinterpretedInt64);
std::cout << d << " -> " << int64Value << " = " << reinterpretedInt64 << std::endl;
}
return 0;
}
You can check it on https://godbolt.org/z/W8oY7fKKT for other compilers
By taking a pointer, reinterpreting based on the pointer and casting back we can reinterpret. The reason I used bit_cast was to avoid the "strict aliasing" warning... see the notes on reinterpret_cast, you will see that the aliasing warning in their sample happens in a situation similar to what we are trying to do, but... since we don't use a local variable but access from an array I don't think the compiler will attempt aliasing. I was trying to play it safe, but seems like I caused a different problem. I suspect reinterpret-cast may be fine in this case.
Maybe its just simpler to use a little utility function such as doubleToInt64 above. Alternatively we use old C++ style pointer casting and if we get a strict aliasing warning we can wrap the block in a conditional to silence the warning because we know what we are doing
Jasper, the latest commit to the export_usingz branch passes all CI testing.
So, pending any tweaks you might suggest, I'd now be happy to merge this back into the main branch.
Edit: Given that the contents of the z_type member is user defined, ISTM that the benefits of a z_type (over the previous int64_t.) are marginal. And Clipper2 users using USINGZ and both integer and float classes/functions will still need to reinterpret cast some z values (unless they compile from completely separate Clipper2 libraries).
Edit2: This Reinterpret function might be better
template <typename T1, typename T2>
inline T1 Reinterpret(T2 value) {
return *reinterpret_cast<T1*>(&value);
}
It all looks good. As a matter of style, I would drop the inferred second template argument to Reinterpret, I usually don't provide types to templates that the compiler can infer.
I noticed that in your Delphi header conversion for the callback you don't have the [ref] keyword with your const arguments that need to be forced to be by reference. The reason I did this because Delphi does magic with const and will pass by value if the type is the size or smaller than some threshold (my guess register size). The [ref] keyword forces it to be passed by reference. If your tests are fine it's probably OK, but if you see issues arise with odd data populated for the vertices this would be my guess. I will probably keep this in our own header conversion.
You can close this pull request without completing it, and then create a pull request from the export_usingz branch into main.