Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

Commit

Permalink
HARP-12773: Roadshield icon disappear, while text remains visible
Browse files Browse the repository at this point in the history
* also fixes infinite render loop due to only part of road shields rendered

Signed-off-by: stefan.dachwitz <[email protected]>
  • Loading branch information
StefanDachwitz committed Nov 19, 2020
1 parent 2183218 commit c071734
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 17 deletions.
13 changes: 2 additions & 11 deletions @here/harp-mapview/lib/text/Placement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const tmpPointDir = new THREE.Vector3(0, 0, 0);
const COS_TEXT_ELEMENT_FALLOFF_ANGLE = 0.5877852522924731; // Math.cos(0.3 * Math.PI)

/**
* Checks whether the distance of the text element to the camera plane meets threshold criterias.
* Checks whether the distance of the text element to the camera plane meets threshold criteria.
*
* @param textElement - The textElement of which the view distance will be checked, with coordinates
* in world space.
Expand Down Expand Up @@ -86,7 +86,7 @@ function checkViewDistance(
* Computes distance of the specified text element to camera plane given with position and normal.
*
* The distance is measured as projection of the vector between `eyePosition` and text
* eonto the `eyeLookAt` vector, so it actually computes the distance to plane that
* onto the `eyeLookAt` vector, so it actually computes the distance to plane that
* contains `eyePosition` and is described with `eyeLookAt` as normal.
*
* @note Used for measuring the distances to camera, results in the metric that describes
Expand Down Expand Up @@ -411,9 +411,6 @@ export function placeIcon(
* @param env - The {@link @here/harp-datasource-protocol#Env} used
* to evaluate technique attributes.
* @param screenCollisions - Used to check collisions with other labels.
* @param isRejected - Whether the label is already rejected (e.g. because its icon was rejected).
* If `true`, text won't be checked for collision, result will be either `PlacementResult.Invisible`
* for newly placed (upcoming) label or `PlacementResult.Rejected` if the label was persistent.
* @param outScreenPosition - The final label screen position after applying any offsets.
* @param multiAnchor - The parameter decides if multi-anchor placement algorithm should be
* used, be default [[false]] meaning try to place label using current alignment settings only.
Expand Down Expand Up @@ -583,9 +580,6 @@ function placePointLabelChoosingAnchor(
* @param env - The {@link @here/harp-datasource-protocol#Env}
* used to evaluate technique attributes.
* @param screenCollisions - Used to check collisions with other labels.
* @param isRejected - Whether the label is already rejected (e.g. because its icon was rejected).
* If `true`, text won't be checked for collision, result will be either `PlacementResult.Invisible`
* or `PlacementResult.Rejected`.
* @param outScreenPosition - The final label screen position after applying any offsets.
* @returns `PlacementResult.Ok` if point label can be placed, `PlacementResult.Rejected` if there's
* a collision, `PlacementResult.Invisible` if it's not visible.
Expand Down Expand Up @@ -632,9 +626,6 @@ function placePointLabelAtCurrentAnchor(
* @param env - The {@link @here/harp-datasource-protocol#Env}
* used to evaluate technique attributes.
* @param screenCollisions - Used to check collisions with other labels.
* @param isRejected - Whether the label is already rejected (e.g. because its icon was rejected).
* If `true`, text won't be checked for collision, result will be either `PlacementResult.Invisible`
* or `PlacementResult.Rejected`.
* @param forceInvalidation - Set to true if text layout or other params has changed such as text
* re-measurement is required and text buffer need to be invalidated.
* @param outScreenPosition - The final label screen position after applying any offsets.
Expand Down
28 changes: 22 additions & 6 deletions @here/harp-mapview/lib/text/TextElementsRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,7 @@ export class TextElementsRenderer {
): boolean {
const pointLabel: TextElement = labelState.element;
const textRenderState: RenderState | undefined = labelState.textRenderState;
const isLineMarker = iconIndex !== undefined;

assert(iconIndex === undefined || labelState.iconRenderStates !== undefined);
const iconRenderState: RenderState =
Expand Down Expand Up @@ -1681,16 +1682,24 @@ export class TextElementsRenderer {
placementStats.numPoiTextsInvisible++;
}
if (!renderIcon || iconInvisible) {
labelState.reset();
// Reset only the iconRenderState for line markers, not the shared
// textRenderState, since some icons may be invisible while others are visible.
// The labelState has to be reset by the calling function if no icons are
// placed to reset the textRenderState properly.
if (isLineMarker) {
iconRenderState.reset();
} else {
labelState.reset();
}
return false;
}
textRenderState!.reset();
}

const iconIsRequired = !(poiInfo?.iconIsOptional === false);
// Rejected icons are only considered to hide the text if they are valid, so a missing icon image will
// not keep the text from showing up.
const requiredIconRejected: boolean = iconRejected && iconReady && iconIsRequired;
const iconIsOptional = poiInfo?.iconIsOptional !== false;
// Rejected icons are only considered to hide the text if they are valid, so a missing
// icon image will not keep the text from showing up.
const requiredIconRejected: boolean = iconRejected && iconReady && !iconIsOptional;

const textRejected = requiredIconRejected || placeResult === PlacementResult.Rejected;
if (!iconRejected && !iconInvisible) {
Expand Down Expand Up @@ -1831,6 +1840,7 @@ export class TextElementsRenderer {

// Process markers (with shield groups).
if (minDistanceSqr > 0 && shieldGroup !== undefined) {
let numShieldsVisible = 0;
for (let pointIndex = 0; pointIndex < path.length; ++pointIndex) {
const point = path[pointIndex];

Expand All @@ -1851,7 +1861,7 @@ export class TextElementsRenderer {
}
}

// Place it as a point label if it's not to close to other marker in the
// Place it as a point label if it's not to close to another marker in the
// same shield group.
if (!tooClose) {
if (
Expand All @@ -1866,10 +1876,16 @@ export class TextElementsRenderer {
)
) {
shieldGroup.push(tempScreenPosition.x, tempScreenPosition.y);
numShieldsVisible++;
}
}
}
}
if (numShieldsVisible === 0) {
// For road shields the shared textRenderState may only be reset if none of the
// icons can be rendered.
labelState.reset();
}
}
// Process markers (without shield groups).
else {
Expand Down

0 comments on commit c071734

Please sign in to comment.