gdal_trace_outline ignores alpha layer about dans-gdal-scripts HOT 4 OPEN

Brannon, Thanks for the patch. I haven't touched this project in years, so it is nice to at the very least have some of it updated to C++11. I think the patches need some work though before being merged. Do the added VC files serve some purpose that will be of use to others? If so, are you willing to be a contact for people who are trying to build on Windows? I know nothing about Windows development, and so can't support this myself and can't judge whether these files really should be included. In common.h, the #include "config.h" has to be up top. In particular, it can't be inside of " #if HAVE_INTTYPES_H" since that symbol comes from config.h. Bring back boost::lexical_cast. It has better error handling than stoi and its ilk in cases of improperly formatted strings. Outputting donuts by default when -major-ring is specified is definitely a change in behavior. Probably it's the right thing to do, because the user can also specify the -no-donuts option if they want. But you need to remove this check: if(major_ring_only && output_no_donuts) fatal_error( "-major-ring and -no-donuts options cannot both be used at the same time"); Please also test that it does the right thing with and without -no-donuts in the case where -major-ring is specified. You can use the images under src/tests to test this. In take_largest_ring, it needs to be " if(mp_in.rings[i].is_hole && mp_in.rings[i].parent_id == best_idx)". And the "if(i==best_idx) continue" is not needed. For consistency, remove spaces before parentheses in "if (" and "for (". Bring back "include <string>" in ndv.h and polygon-rasterizer.cc. These use std::string. Bring back boost::format in raster_features.cc. I think it's clearer than the hexmap[] stuff. The ones that have at most one std::to_string per line are probably more clear with std::to_string though. I don't understand the alpha band logic. Why is it multiplied by 0.25? Shouldn't only full transparency be considered non-data? If the user wants a more fuzzy match, they can specify this with the -ndv option. Is it good to add an NdvSlab when GDALGetRasterNoDataValue fails for some of the bands? I don't have much opinion either way on this, but it should be consistent. As it stands with your current patch, if this call fails for one band then there will be an NdvSlab added only if one of the other channels is alpha. Either it should always add NdvSlab if at least one band has GDALGetRasterNoDataValue, or it should not (regardless of whether another band is alpha). Also, I'm not so sure the logic is sound here. If the file lists [-1,-1,-1] as being NDV (for example), wouldn't you want two NdvSlabs? One for the listed NDV value and another for transparent pixels. As it is now, it'd only be NDV if the pixels were equal to [-1,-1,-1] and also transparent. Instead of the modified line " if(!got_error || alpha_idx >= 0)", it would be better to not set got_error in cases where the slab should be added. Thanks, Dan

Feel free to write up some CMake scripts if you'd like. I'm not particularly tied to autoconf. As long as it still compiles on Linux, it's fine.

config.h comes from autoconf. You can look at config.h.in to see the sorts of things it defines. If all references to these are removed from the code, then config.h will not be needed. A lot of it just detects the existence of headers which pretty much exist on all systems nowadays.

std::stoi also has pretty crummy error detection, unless you pass the pos parameter, and manually check its result. For instance std::stoi("1O1") will return 1 rather than throwing an error. Since we're only using boost headers, and not the compiled libraries, it is probably okay to mix versions.

Yes, if my memory serves, it operates like an AND between the bands. Imagine it as a box in color space. You can review the code to be sure though. It's been several years since I've looked at it. And I'd say by default semi-transparent pixels should be considered valid data. It seems that would be in line with the principle of least surprise. This can always be configured on the command line with the -ndv option.