DM-1128 #6
DM-1128 #6
Conversation
| if (r*r + c*c <= ngrow*ngrow) { | ||
| *row = 8; | ||
| } | ||
| PTR(Footprint) growFootprintRLE( |
There was a problem hiding this comment.
"growFootprintRLE" isn't actually a valid name according to our standards - we require acronyms to be camelcased as well, so it'd have to be "growFootprintRle". That's horrible, admittedly, and an example of why I'm not fond of that particular rule. Given that this is now the only implementation for growing Footprints, maybe just rename it to growFootprintImpl?
|
There are a few code comments above, nothing too serious. Beyond this, I'm a bit surprised that Footprint::normalize() was able to do so much of the work for you with no modification...I take it you verified that this was the case? |
This gives a roughly order of magnitude performance improvement over the convolution with a circle used previously.
mjuric
force-pushed
the
tickets/DM-1128
branch
from
44a189c
to
b0e170f
Compare
| enum class Shape { CIRCLE, DIAMOND }; | ||
| typedef std::vector<Span>::const_iterator const_iterator; | ||
| StructuringElement(Shape shape, int radius); | ||
| StructuringElement(int left, int right, int up, int down); |
There was a problem hiding this comment.
This constructor either needs a fair bit of documentation what it does and what it could be used for, or it should just be removed until we come up with a use case for it.
There was a problem hiding this comment.
Ah, I see that we actually did have a public method for doing this already. I wonder what it was used for. In any case, I suppose it's the job of that method to document what it does, though a pointer from this code to that documentation would be welcome.
This replaces the previous definition, based on std::map, and makes it possible to have asymmetric structuring elements. We can therefore use the RLE-based technique to grow in any (or all) of the up/down/left/right directions.
mjuric
force-pushed
the
tickets/DM-1128
branch
from
28b6259
to
dddc9b3
Compare
New PR for code review of DM-1128, span-based grow operations for Footprint