Narrow and curvy facades, churches and cathedrals

[vvoovv]

Narrow and curvy facades, churches and cathedrals

[vvoovv]

Detecting patterns as the examples in your image is feasible. It would require the unit vector and the length for every edge. The same is required for curvy sequences detection. I propose to add these attributes to BlgEdge.

I added the property length to the class BldgEdge and the property unitVector to the class BldgVector.

[vvoovv]

The code is not optimized at all and its own intention is to provide an experimental base for discussions.

The tangent of the angle between the the edges is already calculated in BldgPolygon.processStraightAngles(..). The calculated tangent can be used later for the pattern detection.

[polarkernel]

I committed a version with improved pattern detection. I replaced the pattern classes spiky and balcony by new classes rectangular and triangular. There are still issues and I am not yet happy with it, but at least it is better than the previous one. Everey time I introduce an improvement, OSM provides an exception somewhere, where the rules don't hold.

As a next step, shared edges have to be excluded, because patterns inside of building groups can't be replaced:

[polarkernel]

I committed a version with improved pattern detection. Exclusion of shared edges is now realized. Small improvements added.

In principle, it would be easy to distinct patterns that go outside of the building (north facades in image below) or inside the building (south facades in image below). Should this be done? This distinction is not possible for curvy facades, as the detector finds also S-shaped facades. I added the osm-file of this building to feature_detection in the repo https://github.com/prochitecture/osm_extracts/ as pattern_test.osm.

There are still situations, where the recognized patterns are not really correct. But it is very difficult to exclude them using the pattern approach. For instance these triangles:

[vvoovv]

I finished integration of the detection of the curved features. The simple features will follow very soon.
To use the feature detection execute:

cd /path/to/blosm
python.exe -m script.command_line --buildings --setupScript setup/mpl_facade_visibility.py --detectFeatures --showFeatures --osmFilepath /path/to/file.osm

[vvoovv]

Integration of the feature detection is finished.

[polarkernel]

That's certainly due to OR in the condition:

Fixed. Uses AND now, as proposed and special condition added for ends of curved sequences. Buildings competely consisting of curved edges are still detetced correctly.

[vvoovv]

Fixed. Uses AND now, as proposed and special condition added for ends of curved sequences. Buildings completely consisting of curved edges are still detected correctly.

I fixed the bug in the conditions that lead to the incorrect detection of the right end of a curved sequence:

Now it's detected correctly.

[polarkernel]

I committed some changes to prevent unneeded character substitution.

I used these characters just for debugging. In principle one could substitute always the same character, for instance an 'X', so that this parameter would not be required for the call to self.matchPattern(..). It has just to be different to all characters used for the edge types.

[vvoovv]

I suggest adding one more value for FacadeClass:

class FacadeClass:
  ...
  narrow = 7

[polarkernel]

A single narrow facades is the one surrounded by wide facades. It has a width below some threshold.

I think this definition is not yet complete. For instance the narrow facades below (manhattan_01.osm, blue arrows) should not be excluded, I suppose:

I assume you think of narrow facades that are more or less perpendicular to the wide facades?

[polarkernel]

Maybe we could discuss some pattern matching. It is easy to implement, I already used it in the bpypolyskel project. Let me define an angle change from the predecessor facade to the current one as smooth, when it is small, say below 30°, or sharp, if it is more (there could also be more ranges, if required). The width of a facade is wide or narrow, using your definition. The combination of angle change and width of a facade can the be expressed by two characters of the following alphabet:

R: sharp right
r: smooth right
L: sharp left
l: smooth left
X: R or L or r or l
w: wide
n: narrow

As starting point, I already found the following patterns (bold and italic part), where ... means repeat previous character zero to N times:

-Xw-Rn-Xw- ==> narrow
-Xw-Ln-Xw- ==> narrow
-Xw-rn...rn-Xw- ==> curvy
-Xw-ln...ln-Xw- ==> curvy

Maybe the complete character sequences of a building polygon could already provided by the class BldgPolygon, as the angles get already calculated there.

[vvoovv]

For instance the narrow facades below (manhattan_01.osm, blue arrows) should not be excluded, I suppose

Why?

I assume you think of narrow facades that are more or less perpendicular to the wide facades?

Not necessarily. A single narrow facade can have an arbitrary angle with a neighbor wide facade.

[vvoovv]

Maybe we could discuss some pattern matching.

I'll review it tomorrow.

[polarkernel]

Not necessarily. A single narrow facade can have an arbitrary angle with a neighbor wide facade.

OK, it seems that I misunderstood your intention. Then also forget about pattern matching, this can be solved using simpler ways. Am I right if I interpret your idea as follows?

All facades with a width below some threshold get classified as narrow. For the classification of side facades: Between a front facade and an unclassified facade, would any number of narrow facades be treated as if they wouldn't exist or does this idea only hold for one single narrow facade? A sequence of narrow facades connecting each other "smoothly" is interpreted as curvy, but does not get its own classification.

[vvoovv]

The reasons to introduce narrow facades is

1. to get some performance gain (visibility isn't calculated for the front facades)
2. more correct detection of side facades in the presence of narrow facades

Would any number of narrow facades be treated as if they wouldn't exist?

That wouldn't work for a sequence of the narrow facades. Let's consider a footprint below:

Suppose the edge at the bottom is classified as the front facade. If we skip the curvy parts, than the edge at the top should be classified as the side facade, but that would be incorrect.

Only single narrow facades should be skipped from the visibility calculation and classification.

[polarkernel]

OK, I think I got it.

Only single narrow facades should be skipped from the visibility calculation and classification.

Just to be sure: I assume this is meant only for their usage to classify side facades. Otherwise narrow facades just stay to be classified as narrow facades, not only single ones but also those that build curvy sequences.

Shall I start to implement this idea?

Would you mind if I add an attribute lengthSquared to the class BldgEdge? In frontOfInvisibleBuilding(), this value is also required for some edges to find the longest edge and could be reused there.

[vvoovv]

Just to be sure: I assume this is meant only for their usage to classify side facades. Otherwise narrow facades just stay to be classified as narrow facades, not only single ones but also those that build curvy sequences.

I suggest to classify curvy facades as curvy

Shall I start to implement this idea?

I want to understand how both narrow and curvy facades fit together before starting the implementation.

[vvoovv]

Example of a footprint without any wide facades and curvy sequences. Only narrow facades with sharp angles are present. Do you think it's possible to simplify that kind of footprint?

[polarkernel]

Do you think it's possible to simplify that kind of footprint?

Should be possible, but I have to do some tests to get some feeling of what the outcome could be.

[vvoovv]

Only narrow facades with sharp angles are present. Do you think it's possible to simplify that kind of footprint?

A building footprint can be completely covered by small elements. But typically there are significant spaces between the small elements like in the cathedrals in Bern:

[vvoovv]

Another example of small elements on the facades. The small elements form balconies in this case.

[vvoovv]

Another example of the small elements on the facades:

[polarkernel]

A building footprint can be completely covered by small elements. But typically there are significant spaces between the small elements like in the cathedrals in Bern:

By simplification, do you mean someting similar as proposed in this paper?

[vvoovv]

By simplification, do you mean something similar as proposed in this paper?

Yes, but I need to formulate more precisely what is needed for. I'll do it today.

[vvoovv]

The issue should be joined with #17

[vvoovv]

In addition to the existing processing (removal of the straight angles) of the building polygons I propose to include:

• pattern matching
• detection of curvy sequences

Churches and cathedrals require special processing.

Examples of the patterns:

A detected pattern is replaced with an edge for the visibility calculation and facade classification. Those proxy edges can be removed again in the second pass of the removal of the straight angles if they form straight angles with their neighbor edges.

It isn't yet clear what to do with the curvy sequences. A possible approach is to measure the length of the edge between the first and the last nodes of the curvy sequence. If the length of that edge is small compared to the building size, then the curvy sequence is skipped form the visibility calculation and the facade calculation. Otherwise it is simplified (like in the paper) and goes through he procedure of the facade classification.

If the whole building polygon is a curvy sequence (or it has only one straight segment) and it doesn't contain an entrance at OSM, then the curve sequence must be simplified (like in the paper) and must go through the procedure of the facade classification. The result of the facade classification must be somehow mapped back to the original curve sequence to find a place for an entrance.

[vvoovv]

I suggest to skip single narrow edges that are not part of the detect patterns from the the facade classification as proposed above.

[polarkernel]

Churches and cathedrals require special processing.

I propose to do the detection of the special parts using the original edges (straight angles already removed) as input and delivering back as output the detected objects (patterns, curvy sequences, maybe others), their position and the replacement proxy edges for visibility calculation and facade classification in one module. This could be computationally demanding. Eventually it could be useful to detect first by a simple test (number of edges?), whether the building is "complicated" and needs special processing, or not and special processing could be skipped.

Detecting patterns as the examples in your image is feasible. It would require the unit vector and the length for every edge. The same is required for curvy sequences detection. I propose to add these attributes to BlgEdge.

The result of the facade classification must be somehow mapped back to the original curve sequence to find a place for an entrance.

The most difficult part in my eyes is a smart embedding of this feature into the current architecture of the program. I do not yet have any idea about the "somehow".

[vvoovv]

I suggest to use another branch for testing the advanced processing.

[polarkernel]

I suggest to use another branch for testing the advanced processing.

New branch dev_patt created for testing the advanced processing.

[polarkernel]

I added the property length to the class BldgEdge and the property unitVector to the class BldgVector.

The function length() does not work. self.v1 and self.v2 are tuples and can't be subtracted. However, I also can't get your intention of what type they should be to have an attribute length in (self.v2 - self.v1).length.

[vvoovv]

I thought they are of the type mathutils.Vector. But they are seemingly not.

[vvoovv]

I'll convert them to either mathutils.Vector or numpy.array.

[vvoovv]

The function length() does not work.

It should work now. Sorry for the bug. It can be accessed as a property:

edge.length

[vvoovv]

I added the file sirius.osm to the osm_extracts. It contains a bunch of curvy buildings and curvy sequences:

[polarkernel]

It contains a bunch of curvy buildings and curvy sequences:

A first result:

The rule for this result was as follows:

• Find patterns of at least 4 consecutive angles between edges, that are in the range of 5° to 30°.

• Collect all edges, that have at least at one end such a low angle and compute the mean length of these edges. This gives an adaptive idea of the length of curvy edges for a building.

• Remove all edges from the sequences, that are longer than a factor (currently 1.5) times the mean length found before. These edges are always at the end(s) of a sequence.

• The remaining edges are curvy edges.

I made a new class FacadePatterns and a first method detectCurvyEdges(). Additionally, I introduced a command line argument --patterns that starts via a do() function this detection, similar to the existing modules. More detection functions will follow to enable for experiments. But the result gets plotted immediately at the end of the function and not yet in a renderer. All this is not yet committed to dev_patt.

I stuck now on how to transfer the detection results to the addon (and to the renderer). Add papttern type to BldgEdge?

[polarkernel]

A surprizing simple result on spike detection. The following simple algorithm works quite well for churches and cathedrals:

• Find patterns of at least 2 consecutive angles between edges, that are larger than 30° to the left, but are not classified as curvy.

• Collect all edges, that have at least at one end such a large angle and compute the mean length of these edges. This gives an adaptive idea of the length of spike edges for a building.

• Remove all edges from the sequences, that are longer than a factor (currently 2.0) times the mean length found before.

• The remaining edges are spike edges.

Unfortunately, this works only for buildings that have at least 50 edges. A simple rectangle would be classified immediately as spike. However, for instance for churches and cathedrals, it works perfectly. A promizing starting point in my opinion (red: curvy, blue: spike):

[polarkernel]

I commited a first version to the branch dev_patt that includes the detection of single narrow facades (I called them spikes) and curvy sequences. The code is not optimized at all and its own intention is to provide an experimental base for discussions. It will require a big refactoring finally. It can be switched on using the command line argument --patterns.

Processing of buildings is filtered by a primitive rule. Only buildings are processed when they have more short edges (< 5m) than long edges or when they have mor than 10 short edges. Currently, all processed buldings are processed as complete buildings, containing the full circular polygon, even if they have shared edges. Maybe buildings with shared edges should be excluded from processing.

The result is painted by a new renderer BuildingPatternRender, where curvy sequences are plotted in red and spikes in blue. It is possible that two patterns overlap, which is not vizualied in the plot. I just added a property pattern to the class BldgEdge to be able to transfer the result to the renderer. But this does not presuppose a future solution.

[vvoovv]

(manhattan_01.osm)

The spike is too large:

[vvoovv]

These edges are always at the end(s) of a sequence.

That statement isn't clear for me. Why are they always at an end of a sequence?

[vvoovv]

I stuck now on how to transfer the detection results to the addon (and to the renderer). Add pattern type to BldgEdge?

I was thinking about a class which instance contains all necessary information information about the sequence (the start and end edges of the sequence, etc). The instance of the class is assigned to the field sequence or pattern of a building edge (BldgEdge). The field is equal to None if the edge doesn't belong to any sequence.

[polarkernel]

That statement isn't clear for me. Why are they always at an end of a sequence?

If the length limit is not requested, there may be edges at the end of such a sequence, that have a length beyond the length limit. Finding this limit is a difficult task and not yet really solved. In the current implementation I have set it to a constant value of 5m. With this value, there still appear edges at the end, that get included to the pattern:

In the comitted code I added an additional rule for patterns that look like balconies:

The rule asks for a long edge (>5m) with an angle of >30° to the left on both ends, that is surrounded by two short edges (angle to left > 30°, short length), similar to the short edges in spikes.

The spike is too large:

This is an edge detected as balcony, as described above. It shows well the issues of a pattern matching approach. There may be unexpected places, that weren't thought for this case, but that match to a pattern.

[polarkernel]

I was thinking about a class which instance contains all necessary information information about the sequence (the start and end edges of the sequence, etc). The instance of the class is assigned to the field sequence or pattern of a building edge (BldgEdge). The field is equal to None if the edge doesn't belong to any sequence.

I am sorry, I do not yet understand your idea. When I look at the whole process, the following steps will be new:

1. A process, similar to what I already proposed, has to detect the small structures like spikes or "balconies". Everey such pattern has a start- and an end-edge.
2. These patterns have to be replaced by a new edge (or new edges?), that adequately may be used for visibility check and classification. The result is a simplified polygon, or a simplified building
3. The relation between the new edges and the original ones has to be stored. This is a n : 1 relation, it can't be stored in the BldgEdge class in my opinion.
4. The new buildings have to be processed for visibility and classification by presenting there a class similar to Building, but containing the simplified polygon.
5. At the end, all edges of this new polygon have a facade class.
6. The facade class of the edges that were replaced has to be transfered somehow to the original edges of the original polygon. The relation stored in 3. may be used for the replaced edges.

In facade_visibility, the function edgeInfo() of the polygon is used to get the edges, while in facade_classification, the function getVectors() of polygon is used. Maybe the instance of polygon could be replaced.

[vvoovv]

These patterns have to be replaced by a new edge (or new edges?), that adequately may be used for visibility check and classification. The result is a simplified polygon, or a simplified building

(1) Patterns with sharp angles

If a pattern is just a small decorative facade element, it can be safely skipped from the visibility calculation.

Otherwise the pattern is replaced by an edge. The proxy edge with its neighbors can be replaced again by an edge if all of them form a straight angle. The resulting edge is classified and its class is propagated to the original edges

(2) Curvy sequences

If the total length of the curvy sequence is significantly smaller than the perimeter of the building polygon, it can be replaced by a single edge for the visibility calculation. If an entrance must be placed on the that sequence, it can be placed in the intersection point of the curvy sequence with a line originating from the proxy edge and perpendicular to the proxy edge.

If the total length of the curvy sequence is relatively large, then it is replaced by a sequence of edges sharing the nodes with the curvy sequence. If an entrance must be placed on the the curvy sequence, it can be done in a similar way as described above,

[vvoovv]

I am sorry, I do not yet understand your idea.

proxyEdge = BldgEdge(edge1.id1, edge1.v1, edge2.id2, edge2.v2)
proxyEdge.prev = edge1.prev
proxyEdge.next = edge2.next
edge1.prev.next = proxyEdge
edge2.next.prev = proxyEdge

sequence = CurvySequence(edge1, edge2, proxyEdge)

currentEdge = edge1
while True:
    currentEdge.sequence = sequence
    currentEdge.skip = PartOfSequence
    if currentEdge is edge2:
        break
    currentEdge = currentEdge.next

[vvoovv]

In the comitted code I added an additional rule for patterns that look like balconies

The structure can be also used for a staircase in a apartments building.

[vvoovv]

The whole code snippet below can be located in the constructor CurvySequence.__init__.

proxyEdge = BldgEdge(edge1.id1, edge1.v1, edge2.id2, edge2.v2)
proxyEdge.prev = edge1.prev
proxyEdge.next = edge2.next
edge1.prev.next = proxyEdge
edge2.next.prev = proxyEdge

sequence = CurvySequence(edge1, edge2, proxyEdge)

currentEdge = edge1
while True:
    currentEdge.sequence = sequence
    currentEdge.skip = PartOfSequence
    if currentEdge is edge2:
        break
    currentEdge = currentEdge.next

[polarkernel]

The whole code snippet below can be located in the constructor CurvySequence.init.

OK, I think I got the idea. But the code does not yet fit to me. BldgEdge has not attributes prev, next and skip. Did you mesan BldgVector? I assume that with edge1 and edge2 you mean the edges that are adjacent to the ends of the pattern's edges.

[polarkernel]

If a pattern is just a small decorative facade element, it can be safely skipped from the visibility calculation.

This must not let a gap between the adges. I assume by skip you mean that the edge before and after the element have to be connected together, without any proxy edge.

[vvoovv]

Did you mean BldgVector?

Yes. Sorry for the confusion.

I assume that with edge1 and edge2 you mean the edges that are adjacent to the ends of the pattern's edges.

vector1 should be instead of edge1, vector2 should be instead of edge2.

vector1 is the initial vector of a sequence.
vector2 is located at the end of the sequence.

[vvoovv]

This must not let a gap between the adges. I assume by skip you mean that the edge before and after the element have to be connected together, without any proxy edge.

The vectors in the sequence are replaced by a proxy vector. The vectors in the sequence are marked as skipped.

[polarkernel]

Before I continue to write code, I like to study the problem in deeper detail and comment my observations and considerations in several posts, starting with this one.

facade_visibility and facade_classification access the edges by the class BldgPolygon. Currently, this class stores the building's polygon by two data structures, an ordered tuple of vectors (which is self.vectors) and as a doubly linked circular list, by the links prev and next of the class BldgVector. This linked list has no anchor.

For visibility detection and classification, these vectors (or their edges) are iteratively accessed either by the methods getVectors() or by edgeInfo(), while the linked list is only used to find neighbors of an edge. Both, getVectors() and edgeInfo() are based on the tuple of vectors.

Inserting a proxy edge is best done using the linked list, as proposed above. But inserting a proxy edge in this list does not change the polygon's output, because the tuple does not get changed and getVectors() and edgeInfo() deliver the original polygon. It would also be costly to insert the new edge in the correct position of the tuple. Therefore I propose to base the polygon completely and only on the linked list and to remove the tuple of vectors.

[polarkernel]

A detected pattern is replaced with an edge for the visibility calculation and facade classification. Those proxy edges can be removed again in the second pass of the removal of the straight angles if they form straight angles with their neighbor edges.

In the current version, the removal of straight edges is handled by declaring them as skipped edges. After that, they never get "unskipped". Removal of proxy edges in the second pass of the removal of the straight angles may lead to complicated code. The whole process is depicted here for a simple detected pattern:

The simplification process including the second pass of the removal is described on the right. Once visibility and facade class are determined, the final edge has to be "unskipped", while the class information has to be transferred back to the original edges. This has only to be done for skipped edges that resulted from proxy edges. This solution is possible, but I doubt that it would be more efficient than to leave the proxy edge where it is for visibility detection and classification.

[polarkernel]

The tangent of the angle between the the edges is already calculated in BldgPolygon.processStraightAngles(..). The calculated tangent can be used later for the pattern detection.

I tried this, but unfortunately it does not work. For edges with sharp angles near 90°, their sign is required. to get the pattern. But the tangent has a discontinuity there, so that for an angle of 89° it is positive, while for an angle of 91° it is negative. The cross product of the unity vectors delivers the sine of the angle, which is continuous at 90°.

[polarkernel]

I commited a cleaned and refactorized version of the pattern detection for simplfication. The main changes are:

• Renamed the main class to FacadeSimplification (new in facade_simplify.py).
• This class is set as first action (before FacadeVisibilityOther), if the command line argument --simplification is set.
• The result of the simplification (currently the detected patterns) gets visualized, when the command line argument --showPatterns is set.
• The methods detectCurvyEdges() and detectSmallPatterns() of the class FacadeSimplification deliver a list of tuples with the first and the last vector of the detected pattern. These vectors can directly be used for a replacement of the pattern.

[vvoovv]

Therefore I propose to base the polygon completely and only on the linked list and to remove the tuple of vectors.

The code from the list comprehension (function(value) for value in iterator) is significantly faster then iteration through the for cycle.

I'll measure the execution time for both variants.

[polarkernel]

The code from the list comprehension (function(value) for value in iterator) is significantly faster then iteration through the for cycle.

You are right. I didn't think on the tasks that maybe are executed more often than insertion. I just thought on complexity of insertions, which is O(N) for ordinary lists and O(1) for linked list (when no search is required).

[vvoovv]

A polygon node with the straight angle can contain the tag for the building entrance. So the related vector skipped due to the straight angle must be preserved.

[vvoovv]

I'll measure the execution time for both variants.

Below is the code I used to measure the execution time. The variant with the ordered list (getVectors1()) is typically 10-20 percent faster than the one with the linked list (getVectors2()). I'll preserve the ordered list. The anchor for the linked list can be added if needed.

import timeit


class BldgVector:
    __slots__ = ("next", "prev", "skip")


class BldgPolygon:
    __slots__ = ("vectors",)
    
    def __init__(self, numEdges):
        vectors = tuple(BldgVector() for _ in range(numEdges))
        
        for i in range(numEdges-1):
            vectors[i].prev = vectors[i-1]
            vectors[i].next = vectors[i+1]
            # skip some vectors
            vectors[i].skip = i%2
        vectors[-1].prev = vectors[i]
        vectors[-1].next = vectors[0]
        vectors[-1].skip = False
        
        self.vectors = vectors
    
    def getVectors1(self):
        return (vector for vector in reversed(self.vectors) if not vector.skip)
    
    def getVectors2(self):
        start = self.vectors[0]
        current = start
        while True:
            if not current.skip:
                yield current
            current = current.next
            if current is start:
                break

polygon = BldgPolygon(10)

print( timeit.timeit("for v in polygon.getVectors1(): v", number=100000, globals=globals()) )
print( timeit.timeit("for v in polygon.getVectors2(): v", number=100000, globals=globals()) )

[vvoovv]

This solution is possible, but I doubt that it would be more efficient than to leave the proxy edge where it is for visibility detection and classification.

I think this solution makes sense for a facade with decorations like on the image below. At least it will reduce the amount of calculations needed for the facade classification.

[vvoovv]

• Renamed the main class to FacadeSimplification (new in facade_simplify.py).

Wouldn't the "feature detection" be a better term in our context?

[vvoovv]

Also Feature or Pattern?

[vvoovv]

A note about feature/pattern classes.

Wouldn't rectangle, triangle, trapezoid be better names for the feature/pattern classes?

[vvoovv]

Each detected feature/pattern should have the attributes width and depth:

[vvoovv]

I created the directory feature_detection in the repo https://github.com/prochitecture/osm_extracts/

File feature_detection/berlin_kraut_strasse.osm:

• An extra edge is added to the detected feature on the left and on the right facades

• A suggest classifying all detected features as rectangle and set the depth attributed for each detected feature.

[vvoovv]

Each detected feature/pattern should have the attributes width and depth:

The attributed width isn't needed. The width of the feature can be taken from the feature proxy edge.

[polarkernel]

Wouldn't the "feature detection" be a better term in our context?

It depends what part of the task shall have priority. I used simplification because one of the tasks is to provide a simplified footprint to visibility detection and classification. When the features itself are used for styles, then "feature detection" is maybe the better term.

A note about feature/pattern classes.
Wouldn't rectangle, triangle, trapezoid be better names for the feature/pattern classes?

The detected parts classified currently as PatternClass.spikes may have quite different forms. Here a small collection:

They are something like bulges, protrusions or convexities. I don't know whether architecture could deliver a term for such building parts.

Each detected feature/pattern should have the attributes width and depth:
The attributed width isn't needed. The width of the feature can be taken from the feature proxy edge.

OK.

[vvoovv]

File feature_detection/tula_bay_window_01:

It contains a number of buildings with a feature in the form of a trapezoid. It represents a bay window.

• An extra edge is added to the detected features.
• Some features aren't detected.

[vvoovv]

When the features itself are used for styles, then "feature detection" is maybe the better term.

I think styling should have a priority. The polygon simplification is a side effect of the feature detection.
So the term "feature detection" should win.

[vvoovv]

Thanks, it works much better now.

I need some time to review and integrate it.

[vvoovv]

File: feature_detection/tula_bay_windows_01.osm

Why is a triangle feature detected for the building edges 410 and 411 and not detected for the building edges 401 and 402?

[polarkernel]

I need some time to review and integrate it.

Take the time. I try to improve the matching, but changes have only to be expected within the functions detectCurvyEdges() and detectSmallPatterns(), while their return values will not change.

Why is a triangle feature detected for the building edges 410 and 411 and not detected for the building edges 401 and 402?

Although they look similar, passing the edges in counter-clockwise order, the patterns are different. I try to ommit this kind of triangle (the same as the one on the right in my last post), it makes no sense.

[vvoovv]

Is it ok if I use the term feature in the code instead of the pattern?

[vvoovv]

the same as the one on the right in my last post

The feature you are referring can represent a balcony. So I suggest to keep the triangle features.

[vvoovv]

Shared edges have to be excluded, because patterns inside of building groups can't be replaced

Yes, they can't be replaced. I think they still have to be detected to apply a correct styling to them.

[polarkernel]

Is it ok if I use the term feature in the code instead of the pattern?

Sure. I will rename also PatternClass to FeatureClass and also all other occurances of this term, for instance detectCurvyEdges -> detectCurvyFeatures and detectSmallPatterns -> detectSmallFeatures.

The feature you are referring can represent a balcony. So I suggest to keep the triangle features.

OK, then the detector requires to find both, edges 410 and 411 and edges 401 and 402. I will fix this.

Yes, they can't be replaced. I think they still have to be detected to apply a correct styling to them.

Do I understand this correctly? The features have to be detected, but a replacement has to be ommitted?

[vvoovv]

Do I understand this correctly? The features have to be detected, but a replacement has to be omitted?

Yes, they should be detected. What do you mean by the replacement? You don't replace the detected features anyway.

[polarkernel]

What do you mean by the replacement? You don't replace the detected features anyway.

There must be a misunderstanding somewhere, because this contradicts your request in this comment:

A detected pattern is replaced with an edge for the visibility calculation and facade classification. Those proxy edges can be removed again in the second pass of the removal of the straight angles if they form straight angles with their neighbor edges.

If we keep polygon simplification as a side effect of the feature detection, as you proposed, then such a replacement of shared edges may lead to conflicts. If in my example on the left of the following image, the detected features get replaced by the red edges, the resulting buildings (at the right) are completely different and do no more have shared edges.

The feature at the bottom with the shared edge should not be replaced. I did not yet realize the replacement in my code, because I didn't know how to do it using the current structure of BldgPolygon.

[vvoovv]

I did not yet realize the replacement in my code, because I didn't know how to do it using the current structure of BldgPolygon.

That what I meant. The replacement isn't yet done in your code.

I'll implement the replacement where it's appropriate.

I'd like to have the features for the shared edges detected as well.

[polarkernel]

I'd like to have the features for the shared edges detected as well.

OK, now I got it, thanks!

[polarkernel]

OK, then the detector requires to find both, edges 410 and 411 and edges 401 and 402. I will fix this.

Fixed. I just committed a new version. Terms with pattern replaced by feature. Shared edges can again provide features.

[vvoovv]

Fixed. I just committed a new version. Terms with pattern replaced by feature. Shared edges can again provide features.

Thanks.

[vvoovv]

Vector length is stored in the array with the index 0 (line 55 of action/facade_simplify.py)

vectorData[:,:2] = [(vector.edge.length, np.cross(vector.prev.unitVector, vector.unitVector)) for vector in vectors]

But in the lines 86-91 the index 1 is used for the vector length:

        # estimate a length threshold
        curvyLengthThresh = np.mean(vectorData[lowAngles,1]) * curvyLengthFactor

        # pattern character sequence. edges with angle between 5° and 30° on either end 
        # and a length below <curvyLengthThresh> get a 'C', else a '0'
        sequence =  "".join( np.where( (vectorData[:,1]<curvyLengthThresh) & lowAngles,'C','0') )

Is it a mistake?

[vvoovv]

Is it a mistake?

I replaced the index to 0 and committed the fix.

[polarkernel]

Is it a mistake?

It was, thanks for checking and fixing. Sorry for late response, I was working too concentrated and didn't check my mails.

[polarkernel]

To use the feature detection execute

Works fine for me. Elegant implementation, I like it!

[vvoovv]

What can be done to prevent the detection of the very wide rectangular features? Perhaps additional conditions for the edge lengths?

[polarkernel]

What can be done to prevent the detection of the very wide rectangular features? Perhaps additional conditions for the edge lengths?

What if we define additionally a maximal edge length to enclose edges classified as 'L' and 'R' in a length range. Maybe replace lines 99 to 105

        sequence = ''.join(
            (
                'L' if ( vector.sin > sin_hi and vector.next.sin > sin_hi ) else (
                    'R' if ( vector.sin < -sin_hi and vector.next.sin < -sin_hi ) else 'O'
                )
            ) 

by something like

        sequence = ''.join(
            (
                (
                    'L' if ( vector.sin > sin_hi and vector.next.sin > sin_hi ) else (
                        'R' if ( vector.sin < -sin_hi and vector.next.sin < -sin_hi ) else 'O'
                    )
                ) \
                if if vector.length < maxLengthThreshold else 'O'
            ) 

[vvoovv]

Wrong detection of a curved feature for the building:

That's certainly due to OR in the condition:

( abs(self.sin)>sin_lo and abs(self.sin)<sin_me ) or ( abs(self.next.sin)>sin_lo and abs(self.next.sin)<sin_me )

If we use AND instead of OR than the edges at the ends of a curved sequence won't be detected. A special condition is required for the edges at the ends of a curved sequence.

[vvoovv]

What if we define additionally a maximal edge length to enclose edges classified as 'L' and 'R' in a length range.

It should set dynamically based on building size. Otherwise we would get the same wrong result for a small building.

[polarkernel]

It should set dynamically based on building size. Otherwise we would get the same wrong result for a small building.

I introduced a dynamically set upper length limit for long rectangular features, based on the maximum size of the bounding box of a building. The length limit is defined as this maximum size times a factor longFeatureFactor, defined in defs. The factor longFeatureFactor is currently set to 0.3, limiting this type of feature to about one third of the building dimension.

Needs maybe some optimization.

[vvoovv]

Needs maybe some optimization.

I made some timeit tests. Your way to calculate the polygon dimension is significantly faster than the alternatives.

[vvoovv]

Why do the feature detection results differ for the rectangular elements of the leftmost part of the building?

[polarkernel]

Why does the feature detection results differ for the rectangular elements of the leftmost part of the building?

This was a very good discovery! It pointed me to a principle bug in pattern matching. The vector sequence started within a feature, at the red dot in the following image:

The character sequence to find matches looked then like this:

<>O<>l<R>l<R>l<R ... >l<R>l<R>l<R>l<R>l<R>l

The repeated character group >l< is the pattern for the rectangular convex feature. Because of circularity, the character sequence is repeated once, so that the feature depicted in the image can be correctly detected at the end of the sequence, (marked by ^^^):

<>O<>l<R>l<R>l<R ... >l<R>l<R>l<R>l<R>l<R>l<>O<>l<R
                                         ^^^

The instruction following matchPattern() has to replace all characters of the rectangular feature by a '1', so that these groups are excluded for subsequent matches:

sequence = re.sub(pattern, lambda m: '1' * len(m.group()), sequence)

This was done correctly for the end of the character sequence, but not for the circular part at the beginning:

<>O<111R111R111R ... 111R111R111R111R111R111>O<111R
^                                        ^^^

The character group <> at the beginnig of the character squence was then detected as concaveTriPattern and produced the error. The bug is fixed now by a test for this special case within matchPattern().

[vvoovv]

Is the default value of the parameter type_char='X' used for the curved features?

[vvoovv]

Substitution of the matched sequence isn't needed in the last call of self.matchPattern(..), is it?

[polarkernel]

Is the default value of the parameter type_char='X' used for the curved features?

It is used just to be able to use the same function matchPattern(), but it is not required for the match.

Substitution of the matched sequence isn't needed in the last call of self.matchPattern(..), is it?

No, it is not required.

[vvoovv]

I committed some changes to prevent unneeded character substitution.

[vvoovv]

Now it's possible to solve the problem of the facade parts classified differently for the example below. Then I'll to proceed to the integration in Blender.