From c071734e7663b51bb16a545aa713ddc8119cf77e Mon Sep 17 00:00:00 2001 From: "stefan.dachwitz" Date: Thu, 5 Nov 2020 10:52:54 +0100 Subject: [PATCH] HARP-12773: Roadshield icon disappear, while text remains visible * also fixes infinite render loop due to only part of road shields rendered Signed-off-by: stefan.dachwitz --- @here/harp-mapview/lib/text/Placement.ts | 13 ++------- .../lib/text/TextElementsRenderer.ts | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/@here/harp-mapview/lib/text/Placement.ts b/@here/harp-mapview/lib/text/Placement.ts index b735249a47..59a8ea434f 100644 --- a/@here/harp-mapview/lib/text/Placement.ts +++ b/@here/harp-mapview/lib/text/Placement.ts @@ -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. @@ -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 @@ -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. @@ -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. @@ -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. diff --git a/@here/harp-mapview/lib/text/TextElementsRenderer.ts b/@here/harp-mapview/lib/text/TextElementsRenderer.ts index 0de31bd0c3..809468c339 100644 --- a/@here/harp-mapview/lib/text/TextElementsRenderer.ts +++ b/@here/harp-mapview/lib/text/TextElementsRenderer.ts @@ -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 = @@ -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) { @@ -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]; @@ -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 ( @@ -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 {