Skip to content

Commit

Permalink
Fix text not being occluded by the globe when overlapmode=always (#4844)
Browse files Browse the repository at this point in the history 
* Add globe text overlap+occlusion render test

* Move render test

* Render test for lines

* Fix overlap: always texts not being occluded by the globe

* Fix icon overlap+occlusion, add render test

* Add changelog entry

* PR feedback
  • Loading branch information
@kubapelc
kubapelc authored Oct 16, 2024
1 parent 28c34a3 commit 5607d32
Show file tree
Hide file tree
Showing 9 changed files with 497 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- _...Add new stuff here..._

### 🐞 Bug fixes
- Fix text not being hidden behind the globe when overlap mode was set to `always` ([#4802](https://github.com/maplibre/maplibre-gl-js/issues/4802))
- _...Add new stuff here..._

## 5.0.0-pre.2
Expand Down
16 changes: 8 additions & 8 deletions src/symbol/collision_index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type PlacedBox = {
box: Array<number>;
placeable: boolean;
offscreen: boolean;
occluded: boolean;
};

export type FeatureKey = {
Expand Down Expand Up @@ -154,12 +155,9 @@ export class CollisionIndex {
const [tlX, tlY, brX, brY] = projectedBox.box;

// Conditions are ordered from the fastest to evaluate to the slowest.
let unplaceable = false;
if (pitchWithMap) {
unplaceable ||= projectedBox.allPointsOccluded;
} else {
unplaceable ||= projectedPoint.isOccluded;
}
const occluded = pitchWithMap ? projectedBox.allPointsOccluded : projectedPoint.isOccluded;

let unplaceable = occluded;
unplaceable ||= projectedPoint.perspectiveRatio < this.perspectiveRatioCutoff;
unplaceable ||= !this.isInsideGrid(tlX, tlY, brX, brY);

Expand All @@ -168,14 +166,16 @@ export class CollisionIndex {
return {
box: [tlX, tlY, brX, brY],
placeable: false,
offscreen: false
offscreen: false,
occluded
};
}

return {
box: [tlX, tlY, brX, brY],
placeable: true,
offscreen: this.isOffscreen(tlX, tlY, brX, brY)
offscreen: this.isOffscreen(tlX, tlY, brX, brY),
occluded
};
}

Expand Down
11 changes: 7 additions & 4 deletions src/symbol/placement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ export class Placement {
let offscreen = true;
let shift = null;

let placed: PlacedBox = {box: null, placeable: false, offscreen: null};
let placed: PlacedBox = {box: null, placeable: false, offscreen: null, occluded: false};
let placedVerticalText = {box: null, placeable: false, offscreen: null};

let placedGlyphBoxes: PlacedBox = null;
Expand Down Expand Up @@ -631,7 +631,8 @@ export class Placement {
placedBox = {
box: placedFakeGlyphBox.box,
offscreen: false,
placeable: false
placeable: false,
occluded: false,
};
}

Expand Down Expand Up @@ -675,7 +676,6 @@ export class Placement {

placedGlyphBoxes = placed;
placeText = placedGlyphBoxes && placedGlyphBoxes.placeable;

offscreen = placedGlyphBoxes && placedGlyphBoxes.offscreen;

if (symbolInstance.useRuntimeCollisionCircles) {
Expand Down Expand Up @@ -809,7 +809,10 @@ export class Placement {
if (symbolInstance.crossTileID === 0) throw new Error('symbolInstance.crossTileID can\'t be 0');
if (bucket.bucketInstanceId === 0) throw new Error('bucket.bucketInstanceId can\'t be 0');

this.placements[symbolInstance.crossTileID] = new JointPlacement(placeText || alwaysShowText, placeIcon || alwaysShowIcon, offscreen || bucket.justReloaded);
// Do not show text or icons that are occluded by the globe, even if overlap mode is 'always'!
const textVisible: boolean = (placeText || alwaysShowText) && !(placedGlyphBoxes?.occluded);
const iconVisible = (placeIcon || alwaysShowIcon) && !(placedIconBoxes?.occluded);
this.placements[symbolInstance.crossTileID] = new JointPlacement(textVisible, iconVisible, offscreen || bucket.justReloaded);
seenCrossTileIDs[symbolInstance.crossTileID] = true;
};

Expand Down
Binary file added ...on/render/tests/projection/globe/text-always-overlap-occluded/line/expected.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
226 changes: 226 additions & 0 deletions test/integration/render/tests/projection/globe/text-always-overlap-occluded/line/style.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
{
"version": 8,
"metadata": {
"test": {
"height": 256,
"width": 256
}
},
"sky": {
"atmosphere-blend": 0.0
},
"zoom": 1,
"glyphs": "local://glyphs/{fontstack}/{range}.pbf",
"projection": {
"type": "globe"
},
"sources": {
"geojson": {
"type": "geojson",
"data": {
"type": "Feature",
"geometry": {
"type": "MultiLineString",
"coordinates": [
[
[
-160,
-25
],
[
-160,
25
]
],
[
[
-140,
-25
],
[
-140,
25
]
],
[
[
-120,
-25
],
[
-120,
25
]
],
[
[
-250,
-25
],
[
-250,
25
]
],
[
[
-80,
-25
],
[
-80,
25
]
],
[
[
-60,
-25
],
[
-60,
25
]
],
[
[
-40,
-25
],
[
-40,
25
]
],
[
[
-20,
-25
],
[
-20,
25
]
],
[
[
0,
-25
],
[
0,
25
]
],
[
[
20,
-25
],
[
20,
25
]
],
[
[
40,
-25
],
[
40,
25
]
],
[
[
60,
-25
],
[
60,
25
]
],
[
[
80,
-25
],
[
80,
25
]
],
[
[
100,
-25
],
[
100,
25
]
],
[
[
120,
-25
],
[
120,
25
]
],
[
[
140,
-25
],
[
140,
25
]
],
[
[
160,
-25
],
[
160,
25
]
]
]
}
}
}
},
"layers": [
{
"id": "background",
"type": "background",
"paint": {
"background-color": "blue"
}
},
{
"id": "text",
"type": "symbol",
"source": "geojson",
"layout": {
"symbol-placement": "line-center",
"text-field": "A",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
],
"text-overlap": "always"
},
"paint": {
"text-color": "yellow"
}
}
]
}
Binary file added ...der/tests/projection/globe/text-always-overlap-occluded/point-icon/expected.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 5607d32

Please sign in to comment.