-
Notifications
You must be signed in to change notification settings - Fork 785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Questions about proj.h #529
Comments
I know that @busstoptaktik is working on some improvements for
It returns
See above.
One sets the error level, the other controls how much noise the program makes. Higher log level, higher output to stdout (or where ever).
I agree that they should be private. We need a place to put them though.
They mainly exists to keep some sort of backwards compatibility. If we can agree on (some form of) my proposal on the mail list to day, we are free to change that behaviour all together, which I think will be a good thing.
|
Also, I agree that something should be done about |
Hum this will need to be documented somewhere. I still don't get how pj_err_level() works and what "error level" means
Yes
Implementation detail then. Candidate for proj_internal.h |
Yeah, there's already a ticket for that: #442. As far as I know Thomas is working on it. Some older un-merged work is here: https://github.com/OSGeo/proj.4/blob/687e591b6d4cdb3370f2bb50fedbda57da83e76c/docs/source/development/obsapi.rst |
Other questions :
Suggestion:
|
The presence of those context explicit functions in proj.h is very surprising, given that the concept itself is absent from other proj.h API. I'm not sure of which use cases they are supposed to cover. I believe they should be removed. |
Good point. So far the ambition has been to use the same underlying objects, but yeah, that might not be possible in the future. Could be fixed with a new prefix as you suggest.
I am -1 on that. At leas those prefixes at least. One of the main ideas with the new API was to create a simple and clean API.
It is not though. For every
Same cases as the existing contexts: Multithreaded applications. |
Yeah, better
For the user of proj API, couldn't this be a hidden implementation detail ? If you consider that the addition of the context concept made the proj_api a bit messy, we should try very hard not to expose it in the new cleaner API.
Show me real-world scenarios that need those functions. Why would I want to "renew" a context, or transfer it to some other objects. For me, the requirements for a multithreaded application are :
Those requirements are what most multithreaded-compatible libraries do |
Yes, it could. It was included in to have a 1-to-1 relationship between proj_api.h and proj.h, in order to ease migration from one to the other. We don't have to go down that road if we plan on keeping proj_api.h as a legacy API.
I have discussed this at length with Thomas, and we are both also confused about why it was made the way it was originally. The only thing that has come up so far, I think, is that it might be useful for several threads to log to the same file, via a shared context. But even that I think is a a highly doubtful use case. Personally I am happy to remove that functionality. |
See http://osgeo-org.1560.x6.nabble.com/threadsafety-and-PROJ-4-the-projCtx-API-Update-td3841784.html . So bascially as I said just to avoid the issue of the global pj_errno
Looking at the above message, I'm pretty sure this wasn't at all in mind of Frank |
I think that is correct. The concern has more been: How has it been used in the wild? So far I haven't seen any surprising uses. The same goes for the FileAPI functions. So I think it will be better to hide contexts and FileAPI in the new API. |
So I think it will be better to hide contexts and FileAPI in the new API.
Being able to do that would be awesome. Kind of like stopping hitting
one self :-)
|
As a summary of all my above remarks, here's a proposal of what I'd consider as a nice proj.h : https://gist.github.com/rouault/890e1c4cf0da5a282c292625fdf20917#file-my-vision-ofa-clean-proj-h Note: I've removed absolutely all includes to other files. |
Can I suggest that you make a new gist, that starts with the current Should we change the prefix to |
Here it is https://gist.github.com/rouault/fe6fa8b610c0c5e15cb3f0c592f36bd5/revisions
I've no definite opinion on this. I've tried a few proj_ likely symbol names with codesearch.debian.net and found that proj_free ( https://codesearch.debian.net/search?q=proj_free ) is actually used by the VTK project. The irony is that this function comes from the original libproj, embedded in VTK sources. But apparently Debian uses a patch to switch to the regular external proj.4: http://sources.debian.net/src/vtk6/6.3.0%2Bdfsg1-5/debian/patches/102_enable_system_proj4_lib.patch/#L115 |
Thanks!
|
Just added PROJ_VERSION_MAJOR, _MINOR and _MICRO to my proposal. prj_ seems a reasonable choice. |
I would prefer proj_ and then using an alternative name for the destructor function - e.g proj_destroy, or proj_delete, being antonyms to proj_create. Whatever name, it would probably just be a thin wrapper for pj_free... |
@rouault - Strange - Frank's post is from june 2010, and elaborates on the problems of implementing thread local storage in a platform independent manner. Just a year later, we introduced platform independent thread local storage for the trlib transformation library (source of proj's transverse mercator and 2D Horner code) using just a single header file to provide some syntactic sugar, and relief for Microsoft-inflicted pains: See comments over at https://bitbucket.org/KMS/trlib/src/eafee76d1127fa1db89ac9c15a9edafabcb599be/TR_INC/trthread.h The idea was to build a single thread library by default, and go thread safe only if the symbol "THREAD" was defined. I realize, though, that PROJ.4 do production runs on many more platforms before lunch, than trlib was ever just marginally tested on (although trlib was intended to run on any platform - including Windows - providing just a bare minimum of posixy blessings). Going forward with a new API I think we should use a combination of an PJ internal error flag, and the system errno. The latter is thread local in threaded systems: It is really a macro expanding to a dereferencing of the return value of a function returning a pointer to the thread local errno (this makes When introducing an errno member of each PJ object, the only remaining need for a global pj_errno is really just to signal that a we could not allocate or initialize the PJ. Cases that may be adequately handled by returning 0 and setting errno to ENOMEM or EINVAL, respectively. Of course setting the PJ->errno should be done using proj-internal macros passing stuff through to the global and context local pj_errnos. Checking validity of a PJ could, at the API level be done by a PJ_OK(P) macro or somesuch (incidentally, "pjok" is the Danish word for "wimp"). We should strive to make the OBS API pj_errno agnostic, widely ignoring the global and context wide pj_errno Since |
Thread local storage requires explicit support from compilers, and at the time this was introduced in proj, I believe this was only for cutting edge compilers, hence the choice for using classic pthread / win32 thread. I wouldn't be enthousiastic about using the standard errno mechanism for proj. errno can be reset by various calls to the standard library, etc in a way that is hard to predict. By using a context-specific pj_errno we master precisely when we set it. |
We should probably remove the following :
Questions :
Suggestions:
The text was updated successfully, but these errors were encountered: