Remove empty labels from being candidates for overlap checking

[JesseSDevaney]

Bug Fix: This PR is intended to update the hideOverlap functionality to prevent empty labels ("") from being considered as candidates for the overlap checking.

• Perhaps there is a better way of ignoring labels with no text, such as setting them to ignore = true in the label creation process and then checking if a label is ignored before doing the overlapping comparison check, but I was unsure if that was the right way to go. I am open to suggestions!

Details

Before: What was the problem?

Empty labels cause non-empty labels to be hidden

hideOverlap = true

hideOverlap = false

After: How does it behave after the fixing?

Empty labels no longer cause non-empty labels to be hidden

hideOverlap = true

hideOverlap = false

Document Info

• This PR doesn't relate to document changes

Others

Merging options

• Please squash the commits into a single one when merging.

[github-actions]

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-19982@205b2b1

[Ovilia]

Please fix the lint problem first.

I don't think it's the correct way to fix this problem by hiding the empty text. It's better to find out why empty labelItem.rect is overlapping with others.

[JesseSDevaney]

Please fix the lint problem first.

• Lint problem fixed: a5aa28a

---

I don't think it's the correct way to fix this problem by hiding the empty text. It's better to find out why empty labelItem.rect is overlapping with others.

If empty labels are not hidden, they will still be considered in the overlapping process (labelLayoutHelper#L311-L364):

1. Empty labels still have a bounding rect, it is just a bounding rect with zero width and zero height - i.e. a point
2. Since they still have a bounding rect, you can still check if they are overlapping - i.e. does a non-empty label have a bounding rect that encompasses the empty labels point?
   • yes -> overlapping
   • no -> not overlapping
3. Taking 1 and 2 together gives us: if an empty label point gets added to the displayedLabels list (see labelLayoutHelper.ts#L328) and a subsequent non-empty label has a bounding rect that encompasses the empty label, it will not be displayed for no reason other then that the empty label got added to the list before it and that there is no check to prevent empty labels from being added to the displayedLabels list

See my graphic below for visual context:

[Ovilia]

Thanks for the detailed explanation! It helped a lot.

Could we change the code of overlapping check algorithm to take special account for those text with zero width or zero height so that they always return false when overlapping checking.

The reason whiy I think we should update the checking algorithm instead of avoid adding text is that we need to make sure this problem fixed in a lower level so that in other places there won't be such cases if we forget to avoid adding empty text, or when the text content is dynamic.

[JesseSDevaney]

Could we change the code of overlapping check algorithm to take special account for those text with zero width or zero height so that they always return false when overlapping checking.

The reason whiy I think we should update the checking algorithm instead of avoid adding text is that we need to make sure this problem fixed in a lower level so that in other places there won't be such cases if we forget to avoid adding empty text, or when the text content is dynamic.

I created a corresponding PR in the ZRender repository, where the overlapping check algorithm is.

• Update BoundingRect intersection check ecomfe/zrender#1081

Would you mind taking a look at that new PR and let me know if this was the kind of fix you were thinking of?

[Ovilia]

After discussing with other members, we tend to believe it's better to make the change here instead of in ZRender because it may be controversial to define whether it's overlapping for an object with empty height/width inside other elements. So we still need to do the change here. I'll close the ZRender PR.

I think the overall logic is great, only that it may make greater sense if we could check the width and height of the text here instead of checking text === ''. And we try to use ?. in ECharts to reduce the compiled size, so label.style && lable.style.text is a better way to check like this.

Thanks for your contribution!

[JesseSDevaney]

Thank you for the update.

I updated the conditional to check the width and height of the bounding rectangle instead of checking the label's text itself.

Let me know if the updated conditional is what you were expecting.

Thank you for your help, Ovilia!

[Ovilia]

globalRect is the unioned bounding rect of the current label and previous labels. So should it be wiser to check labelItem.rect.width === 0 || labelItem.rect.height === 0 instead, before globalRect.copy(labelItem.rect)?