Skip to content

#11775 Validation: Remove layer fixes for building overlaps; recommend level tags #11822

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.

Sign up for GitHub

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

Open
ak8abhinay wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

#11775 Validation: Remove layer fixes for building overlaps; recommend level tags #11822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion data/core.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1811,7 +1811,8 @@ en:
aeroway-waterway:
reference: "Aeroways crossing waterways should use tunnels."
building-building:
reference: "Buildings should not intersect except on different layers."
message: "Buildings should not intersect. If they are vertically stacked, please use the level tag instead of layer."
reference: "Buildings should not intersect. If they are vertically stacked, please use the **level** tag instead of **layer**."
Copy link
Contributor

@matkoniecz matkoniecz Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? If they are in fact vertically stacked buildings then layer is appropriate.

level is for this on different levels (and BTW things on single level may be still vertically stacked)

Copy link
Contributor

@matkoniecz matkoniecz Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is wiki advising this anywhere?

Copy link
Author

@ak8abhinay ak8abhinay Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Key:layer Wiki page advises for buildings in the final para :

'Ways in buildings... should be mostly described with level= instead of layer.'

After looking at the review, I've realized my current code logic is incomplete. While I've updated the message, the validation still incorrectly allows layer as a fix, which is inconsistent. As these changes were made some time ago, I’d like to take some time to properly review and adjust the implementation.

Copy link
Contributor

@matkoniecz matkoniecz Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Ways in buildings... should be mostly described with level= instead of layer.'

it refers to ways in buildings, not to buildings themselves

Copy link
Contributor

@matkoniecz matkoniecz Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure is layer= proper solution of vertically stacked buildings*, but level= is surely not - as it refers to position within a building.

*this is distinct form whether pushing mapper to add fake layer= instead of fixing almost certainly bad geometries is appropriate - hiding "just add layers" when both are buildings makes sense to me

Copy link
Author

@ak8abhinay ak8abhinay Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now. I’ll re-evaluate how overlapping building features should be handled in the validator and adjust the logic accordingly.

building-highway:
reference: "Highways crossing buildings should use bridges, tunnels, or different layers."
building-railway:
Expand Down
46 changes: 33 additions & 13 deletions modules/validations/crossing_ways.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,36 @@ export function validationCrossingWays(context) {
return true;
}

// assume 0 by default; don't use way.layer() since we account for structures here
var layer1 = tags1.layer || '0';
var layer2 = tags2.layer || '0';

if (allowsBridge(featureType1) && allowsBridge(featureType2)) {
if (hasTag(tags1, 'bridge') && !hasTag(tags2, 'bridge')) return true;
if (!hasTag(tags1, 'bridge') && hasTag(tags2, 'bridge')) return true;
// crossing bridges must use different layers
if (hasTag(tags1, 'bridge') && hasTag(tags2, 'bridge') && layer1 !== layer2) return true;
} else if (allowsBridge(featureType1) && hasTag(tags1, 'bridge')) return true;
else if (allowsBridge(featureType2) && hasTag(tags2, 'bridge')) return true;

if (allowsTunnel(featureType1) && allowsTunnel(featureType2)) {
if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true;
if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true;
// crossing tunnels must use different layers
if (hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel') && layer1 !== layer2) return true;
} else if (allowsTunnel(featureType1) && hasTag(tags1, 'tunnel')) return true;
else if (allowsTunnel(featureType2) && hasTag(tags2, 'tunnel')) return true;

// don't flag crossing waterways and pier/highways
if (featureType1 === 'waterway' && featureType2 === 'highway' && tags2.man_made === 'pier') return true;
if (featureType2 === 'waterway' && featureType1 === 'highway' && tags1.man_made === 'pier') return true;

if (tags1.layer !== undefined && tags1.layer === tags2.layer) return false; // Warn if both have the same defined layer

const isElement1Bridge = allowsBridge(featureType1) && hasTag(tags1, 'bridge');
const isElement2Bridge = allowsBridge(featureType2) && hasTag(tags2, 'bridge');
if (isElement1Bridge !== isElement2Bridge) return true; // Either one is bridge, the other is not

const isElement1Tunnel = allowsTunnel(featureType1) && hasTag(tags1, 'tunnel');
const isElement2Tunnel = allowsTunnel(featureType2) && hasTag(tags2, 'tunnel');
if (isElement1Tunnel !== isElement2Tunnel ) return true; // Either one is tunnel, the other is not

return (tags1.layer || '0') !== (tags2.layer || '0');
if (featureType1 === 'building' || featureType2 === 'building' ||
taggedAsIndoor(tags1) || taggedAsIndoor(tags2)) {
// for building crossings, different layers are enough
if (layer1 !== layer2) return true;
}
return false;
}


Expand Down Expand Up @@ -404,6 +419,7 @@ export function validationCrossingWays(context) {
allowsTunnel(featureType2) && hasTag(entities[1].tags, 'tunnel');
var isCrossingBridges = allowsBridge(featureType1) && hasTag(entities[0].tags, 'bridge') &&
allowsBridge(featureType2) && hasTag(entities[1].tags, 'bridge');
var isBothBuildings = featureType1 === 'building' && featureType2 === 'building';

var subtype = [featureType1, featureType2].sort().join('-');

Expand Down Expand Up @@ -475,8 +491,12 @@ export function validationCrossingWays(context) {
featureType1 === 'building' ||
featureType2 === 'building') {

fixes.push(makeChangeLayerFix('higher'));
fixes.push(makeChangeLayerFix('lower'));
// For overlapping buildings, do not suggest changing the `layer=*` tag.
// Vertically stacked buildings should use `level=*` instead.
Copy link
Contributor

@matkoniecz matkoniecz Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least also here

Copy link
Author

@ak8abhinay ak8abhinay Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the check if (!isBothBuildings) here to specifically disable the 'Make Higher' and 'Make Lower' layer buttons for building-to-building overlaps.

By removing these buttons, we stop the editor from suggesting layer as an automated fix and encourage mappers to handle manual fix or fix the geometry. However, I've realized my current logic is a bit inconsistent since the validator still technically allows the tag, so I will review this and come up with proper implementation

if (!isBothBuildings) {
fixes.push(makeChangeLayerFix('higher'));
fixes.push(makeChangeLayerFix('lower'));
}

// can only add bridge/tunnel if both features are lines
} else if (context.graph().geometry(this.entityIds[0]) === 'line' &&
Expand Down