Skip to content
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.

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

Fix Label position height may not be correctly updated when its HeightReference is relative. #11929

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

plainheart
Copy link
Contributor

@plainheart plainheart commented Apr 12, 2024

Description

Hi,

I noticed Label position height doesn't work anymore when its HeightReference is relative since the commit 8e271f1 (diff).

This bug might be caused by a premature assignment to owner._clampedPosition [1]. As the _clampedPosition setter of Label clones the inputted value, the following modification for the owner_clampedPosition won't be applied to the Label glyph billboard [2], which causes the user-specified height not to work as expected. So I moved the assignment to the last line to fix this issue.

[1] https://github.com/CesiumGS/cesium/blob/1.116/packages/engine/Source/Scene/Billboard.js#L1141-L1145
[2] https://github.com/CesiumGS/cesium/tree/1.116/packages/engine/Source/Scene/Label.js#L1155-L1172

Stage Initial After Update
Before Unexpectedly clamped to ground
After

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have update the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

Code snippet for reproduction (Sandcastle)

const labels = new Cesium.LabelCollection({ scene: viewer.scene })
const label = labels.add({
    show: true,
    position: Cesium.Cartesian3.fromDegrees(113.563889, 34.646389, 50),
    text: 'LABEL HEIGHT: 50',
    font: '32px Arial',
    horizontalOrigin: Cesium.HorizontalOrigin.CENTER,
    heightReference: Cesium.HeightReference.RELATIVE_TO_GROUND,
    disableDepthTestDistance: Number.POSITIVE_INFINITY
})
viewer.scene.primitives.add(labels)

setTimeout(() => {
   label.text = 'LABEL HEIGHT: 200'
   label.position = Cesium.Cartesian3.fromDegrees(113.563889, 34.646389, 200)
}, 2000)

…tReference is relative.

- see also previous commit that brought this bug: 8e271f1
Copy link

Thank you for the pull request, @plainheart!

✅ We can confirm we have a CLA on file for you.

@ggetz
Copy link
Contributor

ggetz commented Apr 12, 2024

Thanks for the fix @plainheart! We'll take a look at this shortly.

@ggetz
Copy link
Contributor

ggetz commented Apr 22, 2024

Thanks @plainheart for the fix, and for the testing Sandcastle example making this PR easy to review! I can confirm this addresses the behavior.

@ggetz ggetz merged commit 5475469 into CesiumGS:main Apr 22, 2024
5 checks passed
@plainheart plainheart deleted the fix/label/relative-height-reference branch April 22, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants