Author Topic: OP2MapImager Development  (Read 38296 times)

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #50 on: August 04, 2017, 06:59:04 PM »
Code: [Select]
OP2MapImager ./

I played more with filesystem. filesystem::is_directory returns false upon passing a blank string (""). So, based on this, I'm thinking that filesystem is purposely not iterating over the current directory when a blank string is provided as the path. When I use a dot (.) or ./, everything works properly though. Reading the specification document for filesystem would possibly answer the question definitively, but I'm happy just using ./.

I had forgotten that ./ is used for the current relative directory. :-[ I reworked the OP2MapImager to take either . or ./ for indicating the current directory. Not allowing "" for a relative directory will keep someone from iterating over 100 maps by accident just because they left off the filename by accident. It also plays nicer with filesystem.

It didn't occur to me you could actually pass a blank string or a single space as an argument by using quotes. Experimenting with it a little teased out a bug when you pass in an empty string. I've fixed this bug on the development version.

Some point in the near future I'll release a new version of the OP2MapImager with both of these issues fixed.

So, that simplifies getFilesFromDirectory to the code below (minus the 3 commented out lines in the middle)

Code: [Select]
// For current relative directory use . or ./. An empty string is not considered a valid directory.
vector<string> XFile::getFilesFromDirectory(const string& directoryStr, const string& fileType)
{
path directory(path(directoryStr).remove_filename());

// Brett208 4Aug17: creating a path with an empty string will prevent the directory_iterator from finding any files in the current relative path on MSVC.
//if (directoryStr == "")
// directory = path(".");

vector<string> filenames;

for (auto& directoryEntry : directory_iterator(directory))
{
if (directoryEntry.path().extension() == fileType )
filenames.push_back(directoryEntry.path().string());
}

return filenames;
}

Quote
Have you exported a makefile for the relevant projects?

No, but we will need to decide how to build the project on Linux at some point. I was considering just creating a completely new solution/project in whatever IDE you prefer on Linux and manually configuring required properties. We will have to load in the FreeImage.dll designed for Linux and create a separate post build event that complies with whatever IDE you want to use since I'm doubting it accepts Windows batch scripts. I've read up enough to know sort of what a makefile is and does, but not enough to use in practice. However it is done, I'm sure to learn a lot in the process.

Oh course first the Windows.h header needs to be teased out of the Archive code.

I was thinking to create a Console Application that finished defining the required functions to manipulate archive files first. This way we can test the results of changes to the archive code as windows.h is removed. Then perhaps circle around to the Linux build of everything.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #51 on: August 05, 2017, 08:16:38 PM »
MSVC has an option to export a Makefile. It might be easier to use that than generate a new one by hand. Though the NAS2D makefile is pretty generic, so maybe we could just reuse that.


Teasing out the "#include <windows.h>" will be a bit of a pain. It might be easier if we used regular stream code rather than memory mapped files. Perhaps converting that should be the first step.


I played around with the <experimental/filesystem> code today on Linux. Indeed, the directory_iterator does not like empty path strings. It also doesn't like shell globs, and it doesn't like filenames either. It must be a non-empty folder name.


The filesystem library does handle converting paths between various path/string/char* formats. That could allow some slight simplification to the code. For instance, directory_iterator can take any of those 3 types, which means you don't need to convert a string to a path before calling it. You also don't need to explicitly convert a path to a string before adding it to the vector.

After playing around a bit, I built out my example to be similar to your code, minus the filename extension checks. Here's what it looked like:
Code: [Select]
std::vector<std::string> getFilesFromDirectory(const std::string& dir)
{
  auto pathStr = dir.length() > 0 ? dir : "./";

  std::vector<std::string> filenames;
  for (auto& entry : fs::directory_iterator(pathStr))
  {
    filenames.push_back(entry.path());
  }

  return filenames;
}


Oh, and I'd say just assume the folder name passed is an actual folder, and not a file. Trying to strip a filename from the path and use the containing folder is a bit unexpected and could lead to subtle bugs in library use. I'd say just let the path feed through to the directory_iterator, where it will cause an exception if it refers to a file rather than a folder.

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: OP2MapImager Development
« Reply #52 on: August 05, 2017, 09:09:19 PM »
Would it make sense to use something like PhysicsFS to handle the cross-platform file handling? I've used it with great success.

Though it may make more sense to grab the code from TrenchBroom (map editor). It has an abstract filesystem that seems to have everything for Windows/Linux/Mac and it works pretty well once you get the hang of working with it:

https://github.com/kduske/TrenchBroom/blob/master/common/src/IO/FileSystem.h
https://github.com/kduske/TrenchBroom/blob/master/common/src/IO/FileSystem.cpp

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #53 on: August 05, 2017, 10:02:44 PM »
Upon cursory inspection, it looks like a nice API. Though I tend to shy away from adding external dependencies when they can be avoided. It complicates distribution and compiling.

I suppose that's one of the nice things about the Ruby world, or NodeJS. They have their own language specific packaging and distribution system, and the language core libraries insulate most code from platform considerations, so source is often highly portable between systems. It makes it very easy to incorporate external code, without adding significant difficulty for distribution, compiling, or platform considerations.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #54 on: August 06, 2017, 05:52:26 AM »
leeor_net,

It looks like a clean library. The goal of using filesystem is that it may eventually become part of the standard library, which means many people would become familiar with it. Of course, since it is experimental right now, maybe using it isn't worth the squeeze.



Hooman,

Are you using an alias of fs for the namespace std::experimental::filesystem? If not, it looks like they named the namespace differently between the two compilers.

Code: [Select]
for (auto& entry : fs::directory_iterator(pathStr))
Produces

Error (active)   E0276   name followed by '::' must be a class or namespace name   



I'm getting E0304 from the line of code below. If I change it to entry.path().string(), then it compiles. So it looks like there are some differences between the two compilers implementation.

Code: [Select]
filenames.push_back(entry.path());
Error (active)   E0304   no instance of overloaded function "std::vector<_Ty, _Alloc>::push_back [with _Ty=std::string, _Alloc=std::allocator<std::string>]" matches the argument list

Here is the function rewritten to work in VS17. Since I have a using statement, declaring experimental::filesystem namespace for directory_iterator isn't necessary, but left it to point out the difference.

Code: [Select]
std::vector<std::string> getFilesFromDirectory(const std::string& dir)
{
auto pathStr = dir.length() > 0 ? dir : "./";

std::vector<std::string> filenames;
for (auto& entry : experimental::filesystem::directory_iterator(pathStr))
{
filenames.push_back(entry.path().string());
}

return filenames;
}

Also, if we want to accept empty strings as meaning the same thing as ./ or ., we should update the following two functions to read this way.

Code: [Select]
bool XFile::isDirectory(const string& pathStr)
{
if (pathStr.length() == 0)
return true;

return is_directory(pathStr);
}

Code: [Select]
bool XFile::pathsAreEqual(string pathStr1, string pathStr2)
{
if (pathStr1.length() == 0)
pathStr1 = "./";

if (pathStr2.length() == 0)
pathStr2 = "./";

path p1(pathStr1);
path p2(pathStr2);

return p1 == p2;
}

If this all looks good, I'll commit the changes.

-Brett
« Last Edit: August 06, 2017, 05:55:22 AM by Vagabond »

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #55 on: August 06, 2017, 11:51:33 AM »
Ahh yes, I forgot the namespace line:
Code: [Select]
#include <experimental/filesystem>
namespace fs = std::experimental::filesystem;


The blank path was a problem for directory_iterator. I don't know if it's an issue for is_directory or path object comparisons. I had no trouble setting a path object to the empty string. I only had problems when I tried to use an empty path object with directory_iterator.

Maybe check what happens when is_directory is passed an empty string. As for comparing path objects, I'm wondering how robust the comparison is. A simple string compare won't be very effective in some cases, though I don't know if it does a string compare or something more involved. What if you compare "." and "./"? What if you compare "./" and "./subFolder/../"? In some cases it may make sense to do a simple string compare, to see if two paths are the same and in the same form. In other cases, you might want to know if two paths refer to the same thing, even if they are given in different forms.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #56 on: August 06, 2017, 08:18:00 PM »
Just committed changes to the repo.

There are now 3 versions of getFilesFromDirectory. The one Hooman just wrote, one using regex for the filename, and one that uses the file extension. The regex and file extension overloads first call the generic getFilesFromDirectory and then remove files that do not meet the regex or file extension requirement. This way we do not have to repeat the line of code forcing a blank string into "./" and the directory iterator in each function.

Code: [Select]
vector<string> XFile::getFilesFromDirectory(const string& directory, const regex& filenameRegex)
{
vector<string> filenames = getFilesFromDirectory(directory);

for (int i = filenames.size() - 1; i >= 0; i--)
{
if (!regex_search(filenames[i], filenameRegex))
filenames.erase(filenames.begin() + i);
}

return filenames;
}



I have already tested is_directory with a blank string. It will return false, which makes sense in light of the directory_iterator not working with a blank string.



I played some with comparing paths. Using == against filesystem::path basically compares if the underlying string is the same. So == will return "." and "./" as different (false). However, the filesystem::equivalent function will return "." and "./" as being the same (true).

So, I rewrote the function as pathsAreEquivalent. The areas I was using pathsAreEqual in the code base, I was using it to compare if the filename itself was the same. Now in the code base, I just changed it to compare (==) the raw strings or used strcmp in the archive code since it uses char* instead of std::string.

Code: [Select]
// Will throw an exception if passed only a filename as path1 or path2. Lead filenames with relative or full directory.
bool XFile::pathsAreEquivalent(string path1, string path2)
{
if (path1.length() == 0)
path1 = "./";

if (path2.length() == 0)
path2 = "./";

return equivalent(path(path1), path(path2));
}

Test Data:

Code: [Select]
string d1 = ".";
string d2 = "./";
string d3 = "./Debug/../";
string d4 = "";

bool equal = XFile::pathsAreEquivalent(d1, d2); // TRUE

equal = XFile::pathsAreEquivalent(d1, d3); // TRUE

equal = XFile::pathsAreEquivalent(d2, d3); // TRUE

equal = XFile::pathsAreEquivalent(d1, d4); // TRUE
« Last Edit: August 07, 2017, 06:51:21 AM by Vagabond »

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #57 on: August 07, 2017, 07:03:08 AM »
Just committed code again. Fixed bugs when creating pathsAreEquivalent function. I got bit by trying to use == against 2 char* in the archive code since it uses char* instead of string. Ran out of time to fix when working last time so it was in a bit of an awkard state. Everything is working now.



We need to consider how to handle filenames being the same on different operating systems. For example on Windows tutorial.map is the same as Tutorial.map, but they are different valid filenames on Linux I think? So, simply comparing 2 filenames using == or strcmp may not be appropriate.

I think we want manipulated files to remain compatible on both systems. If on Linux you create tutorial.map and Tutorial.map and then pack them both into maps.vol, Windows will not be able to tell them apart. So really, the code should use the more restrictive option and treat tutorial.map and Tutorial.map as the same to prevent stuff like this from happening. It may not be initially intuitive to a Linux user at first, but we can comment the code to indicate the differences.

I've removed parthsAreEqual from XFile. In retrospect, do we want to re-add the function and use it to force Linux to consider ./tutorial.map as an equal path to ./Tutorial.map?

-Brett

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #58 on: August 08, 2017, 07:47:48 AM »
Quote
There are now 3 versions of getFilesFromDirectory. The one Hooman just wrote, one using regex for the filename, and one that uses the file extension. The regex and file extension overloads first call the generic getFilesFromDirectory and then remove files that do not meet the regex or file extension requirement. This way we do not have to repeat the line of code forcing a blank string into "./" and the directory iterator in each function.

Ahh, I wasn't expecting you to snaffle my function. That's fine. Sounds like you've simplified the code. Though I should point out it could be considerably less efficient to build the full file list, and then filter it to remove items you don't want. That is especially true of a large folder with only a few files of interest.

Seems like a template function could be written that takes some sort of filter function. That could eliminate code duplication, runtime costs of building a list and then filtering, and also dispatch cost for the filter function. A default filter function could be set that doesn't remove any filenames. Not quite sure how it would work though.

Code: [Select]
return equivalent(path(path1), path(path2));
What is "equivalent"?

I like how you have test code. Is it wrapped in a test suite? I was thinking having a few unit tests could be good for some of these projects.


The case sensitivity thing is more an issue of the file system than the OS. Think how Linux accesses NTFS or FAT partitions, or what happens when Windows accesses a file over a network share that's hosted on Linux. Sometimes there is a bit of support to paper over differences, sometimes not.

The VOL files are essentially their own file system. Whether they are case sensitive or not is up to the design of VOL files. I suppose I could check the Outpost 2 code to see if it uses case insensitive comparisons. Mostly though I'd suggest not bothering. It's not likely to be an issue in practice, and if it is, the user will probably be the best person to handle it.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #59 on: August 09, 2017, 06:48:02 AM »
Equivalent means the two supplied paths return the same underlying directory or filename.

I can do generics in C#, but templates in C++ are beyond my current skill without some more studying/research. I'm not opposed to learning it though. I agree that it could be inefficient, but it also isn't noticeably affecting the execution speed of any of the consuming code.

I'll try not to get too wrapped up in case insensitivity then.

For testing the equivalent function, I just stuffed some code in the main function and stepped through it in the debugger to record the BOOL results. I'm not opposed to setting up unit tests, but I think typically they are done through some sort of framework. I have only done a little unit testing and that was all through C#. I guess we would need to discuss the strategy we wanted to use. I like that D has unit test built into the core of the language specification.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #60 on: August 09, 2017, 08:37:34 AM »
Quote
Equivalent means the two supplied paths return the same underlying directory or filename.

That feels vague. How is it determined? If "a" is a symlink to "b", does 'equivalent(path("/path/to/a"), path("/path/to/b"))' return true?

You're right about the inefficiency not affecting the program in question. There are bigger fish to fry.

I've never really done much unit testing in C++, at least not what I would call proper unit testing. Some cursory searching reveals there is a Google Test library that seems to be quite popular. It might be good to start developing a culture of unit testing here. Seems recent projects are taking off, and more people are starting to collaborate. Unit testing would be good for that.

Thought to bring this all back from things we could, or maybe should do, what are the specific objectives for the project. Maybe some of this is a bit of a distraction.

It seems the OP2MapImager is a basically working and complete project. Do you want to add more features, continue polishing it, or move on to something else? All are valid options.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #61 on: August 10, 2017, 06:20:22 AM »
Quote
That feels vague. How is it determined? If "a" is a symlink to "b", does 'equivalent(path("/path/to/a"), path("/path/to/b"))' return true?

I deleted the equivalent function and replaced it with the pathsAreEqual function. I was getting frustrated with the filesystem::equivalent function because it throws errors if the path doesn't actually exist on the hard drive. pathsAreEqual ignores case and treats "./tutorial.map" and "tutorial.map" as equal. So now if you type "./Tutorial.map" it will recognize that you want to render "tutorial.map". It also won't throw an error if you are comparing filenames that only exist in an archive files. I didn't really have any use cases for pathsAreEquivalent anyways.

Quote
Thought to bring this all back from things we could, or maybe should do, what are the specific objectives for the project. Maybe some of this is a bit of a distraction.

Besides re-releasing the map maker with these bug fixes, I just want to get the OP2Utility code to a point where you are happy with its state when we are ready to port it to Linux. OP2Utility is also being used by OP2Archive. Otherwise, I'm finished with the application until someone has a problem with it.

If we are serious about unit testing than we can either use this project as a test case or start by hooking it into OP2Archive (sorry for the pun :) ). I'll check out Google Test in the near future. If it is popular, then it is probably a good choice because it should be supported for a long time, people may already be familiar with it, and learning it may help directly apply to projects outside of Outpost 2.

Leeor,

I don't suppose OutpostHD/NAS2D uses any unit test library???

Also, I'll be uploading my changes shortly to the REPO. I kind of fell behind on this.

-Brett

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: OP2MapImager Development
« Reply #62 on: August 10, 2017, 03:43:16 PM »
I don't suppose OutpostHD/NAS2D uses any unit test library???

No -- I never got around to it. What I've been doing instead is building small programs that test core functions/classes/objects and confirming output/reliability either visually (with the renderer test, test2 in NAS2D-Tests) or with text output.

NAS2D's test programs are here: https://github.com/lairworks/nas2d-tests

The Renderer test (test2) is the most complete test though there are other functions I haven't added to it yet to inspect their output.

For OutpostHD, I built small sample programs that test specific parts of the simulation. There are three at the moment, the first and second revisions of the Population model and the PopulationPool object to ensure correct behavior: http://redmine.outpostuniverse.org/projects/outposthd/files

I don't know if these could be considered 'unit tests' or if they'd even be helpful. But, there they are. :)

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #63 on: August 12, 2017, 04:55:52 AM »
Quote
I don't suppose OutpostHD/NAS2D uses any unit test library?

I was thinking similarly.

Leeor_net, nice to see those tests. I didn't even notice they existed before. I took a quick peek. They're not quite unit tests, in that results are presented in a MsgBox. Usually unit tests have an automated pass/fail criteria, and console logging of the test results. This makes it easy to have them run automatically, often after every commit, or pull request, and the results logged to be inspected later. Inspecting the output would be similar to the Travis CI or Codacy output setup for the GitHub repo.

In fact, such tests could be run by the Travis CI build. Though, usually unit tests get added to the same repo, which makes the process a little easier. Makes sense though, since unit tests are usually added and updated along with the code they are testing. You can keep the tests out of the "src" folder though, perhaps in a separate "test" folder. The build system doesn't need to build the test code by default.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #64 on: August 12, 2017, 06:51:01 PM »
I spent some time reading through Google Test's documentation.

The documentation seems clear to read. It seems like robust support for both Windows and Linux, although based on some of the documentation, I tend to think it wasn't developed for Visual Studio primary (which is fine). I liked how they pointed out in their documentation where they differed some from a standards document on unit testing and that the standards were in fact stated better than what they had developed. Shows a healthy respect for standardization.

It also seems pretty in line with the ideas/theories that I've used in my limited C# based unit testing in the past.

I'll look into downloading the Linux and Windows builds and see how big the downloads are, etc. I'd probably prefer pushing this into OP2Archive first over back-fitting OP2MapImager though.

Happy Coding!

-Brett

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #65 on: October 09, 2017, 09:37:39 AM »
Hey everyone,

I'm working towards another release of OP2MapImager. There are a couple of bugs that need to be fixed. Also, I'm ensuring the code base still aligns and works with all the changes I've made to the OP2Utility project. Lastly, working on OP2Archive I've learned some better practices specific to C++ that I'm pushing into the OP2MapImager code base.

Changes to this point are:
  * Fixed crash if supplying an empty variable ("") for a command via the command prompt.
  * Fixed method to provide current directory to use either '.' or './' instead of '/' or '\'.
  * Made the function XFile::replaceFilename platform agnostic.
  * Fixed typos in ReadMe.txt.
  * Removed namespace using statements from header files.
  * Remove Microsoft specific exception classes.
  * Removed Microsoft specific for each loops and replaced with C++ standard range loops.

Future Work:
  * Encapsulate ConsoleArgumentParser in a class instead of a namespace. (This should probably be an inherited class from the OP2Utility project that is shared with the OP2Archive, but I think this will be outstanding for now as it is a bit complex on how the inheritance would work.)
  * Do some general bug cleaning.
  * Quick review of ReadMe.

Depending on how many bugs crop up, it could be a fairly short time before releasing. I want the code to be in a good place in case anyone decides to further it.

I spent some time trying to get FreeImage source code to statically compile against OP2MapImager. This would allow for removing the DLL from the distribution. Unfortunately, it is not a trivial task and sort of defeated me.

-Brett

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: OP2MapImager Development
« Reply #66 on: October 10, 2017, 07:10:26 AM »
Nice work! C++ is a difficult language to master, for sure, but you're doing great! :D

On my next day off I'll poke around and see if I can get FreeImage to statically link against the project. Dynamic linking has its merits but it can be painful when multiple versions of a library are available and then there's the distribution and so on.  :-\

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #67 on: October 12, 2017, 01:29:23 AM »
Might want to check the licensing to see if there are any restrictions concerning static linking. There are sometimes differences between static linking and dynamic linking. Probably not an issue if you've posted the source code publicly, though you may need to ensure some licensing information is distributed with any compiled executables that contain statically linked libraries.

You're right though in that static linking does make distributions easier and less error prone.

And yeah, you have done an amazing job on this stuff.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #68 on: October 12, 2017, 07:01:34 AM »
Leeor and Hooman,

Thanks!

FreeImage is under a GNU General Public License, version 3 (GPL-3.0). See attached text document. I'm already bundling it with the executable for the program. I'm pretty sure that they don't care about static linking, but if anyone else interprets it differently, let me know.

I found a bug that throws an error whenever you attempt to image a map not in the current working directory of the application. It works properly when accessing the map from an archive file, and just seems to manifest when it is a loose map. After that is fixed, it should be ready to distribute.



Concerning FreeImage static compilation
I can get FreeImage to compile into a library DLL. I downloaded the source and had to modify it in a few places as described here: https://sourceforge.net/p/freeimage/patches/108/. This fix seems to be required when using the default compiler with VS2015 or VS2017.

After this, I tried to link the FreeImage Visual Studio project into the OP2MapImager solution. This caused lots of problems as FreeImage contains about 13 different projects besides the primary one.

I stumbled on an article on Stack Overflow explaining how to statically link Free Image: https://stackoverflow.com/questions/16186300/linking-freeimage-as-a-static-library-in-vs2010.

It looks like the author is compiling FreeImage for static use standalone (not brought into the primary application as a sub project). Then they are linking to the resulting compiled code.

One of the goals of statically linking to me was to try and reduce the distribution size as FreeImage.dll is about 5mb. I'm not sure if statically linking would actually reduce the size or not? I was hoping static linking would just throw out any parts of the library I'm not using in the final build, but I don't have any experience with this in C++.

-Brett

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: OP2MapImager Development
« Reply #69 on: October 12, 2017, 11:01:31 PM »
Static linking would dramatically reduce the size as the only code that would be linked is code that is used. Since you're not using every function in the library (probably just a handful of public and private functions) it should be considerably smaller. Kinda like NAS2D -- it's static library is 30 something MB but linked binaries are way, way smaller.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #70 on: October 13, 2017, 04:29:56 AM »
https://www.gnu.org/licenses/gpl-faq.en.html#GPLStaticVsDynamic:
Quote
Does the GPL have different requirements for statically vs dynamically linked modules with a covered work? (#GPLStaticVsDynamic)
No. Linking a GPL covered work statically or dynamically with other modules is making a combined work based on the GPL covered work. Thus, the terms and conditions of the GNU General Public License cover the whole combination. See also What legal issues come up if I use GPL-incompatible libraries with GPL software?

Does the LGPL have different requirements for statically vs dynamically linked modules with a covered work? (#LGPLStaticVsDynamic)
For the purpose of complying with the LGPL (any extant version: v2, v2.1 or v3):

(1) If you statically link against an LGPL'd library, you must also provide your application in an object (not necessarily source) format, so that a user has the opportunity to modify the library and relink the application.

(2) If you dynamically link against an LGPL'd library already present on the user's computer, you need not convey the library's source. On the other hand, if you yourself convey the executable LGPL'd library along with your application, whether linked with statically or dynamically, you must also convey the library's sources, in one of the ways for which the LGPL provides.

https://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.en.html:
Quote
Linking ABC statically or dynamically with other modules is making a combined work based on ABC. Thus, the terms and conditions of the GNU General Public License cover the whole combination.

Sounds like your code would also have to be covered by the GPL if it makes use of GPL libraries.


Personally, I find GPL a bit restrictive.

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #71 on: October 13, 2017, 10:31:45 AM »
Thanks Hooman.

Does anyone have a problem if we push the OP2MapImager under a GPL license then?

I was just looking to leave everything open source, which I think this would fit the bill.

-Brett

Offline leeor_net

  • Administrator
  • Hero Member
  • *****
  • Posts: 2350
  • OPHD Lead Developer
    • LairWorks Entertainment
Re: OP2MapImager Development
« Reply #72 on: October 13, 2017, 03:00:24 PM »
As much as I dislike the GPL I see no reason why we couldn't do that with OP2MapImager. It's open source after all. I do prefer MIT/BSD/Zlib licenses but to make it easy I would just GPL it.

Offline Hooman

  • Administrator
  • Hero Member
  • *****
  • Posts: 4954
Re: OP2MapImager Development
« Reply #73 on: October 16, 2017, 06:29:37 AM »
Quote
Does anyone have a problem if we push the OP2MapImager under a GPL license then?

My first thought it, publish your code under whatever license you want.


My second thought was, oh crap, your project includes my code too. How does that work? Copyright law is confusing. This is an area I've been avoiding, not wanting to get into the details of. I suppose I've been kicking this can down the road for years.



It seems with code, a copyright is automatically assigned to the author, and only the author can grant a license. It also sounds like the author is free to distribute under any license they choose, even multiple licenses at the same time, and to change which licence they distribute under. What license is chosen by the original author does not restrict how the original author may distribute code in the future. The license only really restricts or protects third parties.

The complications come when combining code from multiple authors. There the various licenses can get quite restrictive. In particular, the GPL, and other "copyleft" or "share-alike" software. Basically, third parties are allowed to modify and redistribute, but must also grant that permission to others. The combination as a whole must be redistributed under a compatible license. If you want to use both a proprietary library, and a GPL library, you're in trouble, since the GPL states the combination must be redistributed under the GPL, while the proprietary library says that library can't be redistributed. This is in contrast with "permissive" licenses, which do allow such combinations.


Quote
As much as I dislike the GPL I see no reason why we couldn't do that with OP2MapImager. It's open source after all. I do prefer MIT/BSD/Zlib licenses but to make it easy I would just GPL it.

I agree. Personally, I find the GPL to be rather restrictive. I prefer not to encumber people. My preference is something closer to public domain. Though I've heard declaring something as public domain has its own issues, and isn't legally recognized everywhere.

The Wikipedia article on Permissive Software Licence has a nice diagram under the License Compatiblity section. It makes the MIT and BSD licenses look attractive.

Also, love the quote in that article
Quote
The way it was characterized politically, you had copyright, which is what the big companies use to lock everything up; you had copyleft, which is free software's way of making sure they can't lock it up; and then Berkeley had what we called ‘copycenter’, which is ‘take it down to the copy center and make as many copies as you want.’

— Kirk McKusick, BSDCon 1999


In terms of license compatibility, there are a number of licenses that are compatible with the GPL. What this means is a project can re-use code that has one of those licences, and combine it into a work, with GPL code, and the result can be redistributed under the GPL. It does not mean the combination can be redistributed under the other licenses. In particular, the GPL typically prevents this.

GPL code can use BSD code, but BSD code can not use GPL code.


What all this seems to say, is that you have to licence your code with the GPL if you want to use the Free Image library with it. It seems you have no choice for your project. I can choose to license my code, which you're re-using in a library-like manner, separately and under a difference license. But in terms of the combination, the combination must be covered by the GPL. (Which is only possible if I choose a compatible open source license).


Well, that was many hours of fairly dry reading.  :P
« Last Edit: October 16, 2017, 06:32:27 AM by Hooman »

Offline Vagabond

  • Global Moderator
  • Hero Member
  • *****
  • Posts: 1013
Re: OP2MapImager Development
« Reply #74 on: October 16, 2017, 09:49:18 AM »
Hooman,

Thanks for doing the research. I'm just looking for the code to remain available for other people to review, edit, and compile as they see fit without assuming responsibility for any perceived damages the source code or compiled application could cause. I'm willing to take your lead and just assign the same license on my portion of the code as long as what you pick fulfills this.

It sounds like statically linking to FreeImage does not fit the bill with what you would be comfortable with. So in the meantime we can just leave it in DLL form (which I think we can do and license or not license the rest of the code as we see fit). This has the added benefit of being less work.

We could also search for a different image manipulation library. FreeImage is only integrated into 2 files in the project, RenderManager.h and RenderManager.cpp. We are using FreeImage to open, scale, create composite images, and save in popular image formats. These tasks should be available to any mature image manipulation library. So, we could search for a different library that had a more agreeable license to you and then integrate it instead. This isn't something I'm really interested in putting time into right now. It wouldn't bother me is someone wanted to put the time in though.

However, FreeImage seems in my limited experience to be one of the most mature and used open source C++ image manipulation libraries.

Bottom line for now, I'll stop trying to statically link FreeImage. If you want to choose a license, I'll check it out and make sure it is agreeable on my end. If not, we can just leave things as they are for now. It might be a while, but when I have some free time, I'll try to check out the MIT and BSD licenses sense that seems to be what you are leaning towards.

-Brett