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

Fixed Hidden Point Primitive Bug #8054

Merged
merged 6 commits into from
Aug 16, 2019

Conversation

ProjectBarks
Copy link
Contributor

create an entity with point graphics and change its show property to false it doesn't hide, instead, it positions itself in the center of the screen while still being visible.

this issue only occurs when I'm in a Ubuntu environment

Fixed by changing translucency in addition to point location when showing

This is in response to #8043

create an entity with point graphics and change its show property to false it doesn't hide, instead, it positions itself in the center of the screen while still being visible.

 this issue only occurs when I'm in a Ubuntu environment

Fixed by changing translucency in addition to point location when showing
@cesium-concierge
Copy link

Thank you so much for the pull request @ProjectBarks! I noticed this is your first pull request and I wanted to say welcome to the Cesium community!

The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.

  • ❌ Missing CONTRIBUTORS.md entry.
  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

Added changes to Changes.MD and added myself to Contributors
@OmarShehata
Copy link
Contributor

Some thoughts:

Moved logic for point size to the end as well as changing the size and position in addition to the color

Sean suggested this will cause the point to be disgarded before moving on to the fragment shader
@ProjectBarks
Copy link
Contributor Author

Thanks omar! Good suggestions.

@emackey
Copy link
Contributor

emackey commented Aug 13, 2019

Billboards are similar to points, do they have the same issue?

@ProjectBarks
Copy link
Contributor Author

@emackey it doesn't appear to be an issue. Tested using this sandbox.

CONTRIBUTORS.md Outdated Show resolved Hide resolved
@mramato
Copy link
Contributor

mramato commented Aug 14, 2019

I can confirm this is fixed on my machine. Thanks @ProjectBarks this one really annoyed me.

@OmarShehata I'll let you handle the rest of this PR, merge when you are ready (or bump to Sean if you need a second opinion).

@mramato
Copy link
Contributor

mramato commented Aug 14, 2019

There are a bunch of local test failures in this branch. @ProjectBarks you probably aren't aware of this, but travis CI doesn't run the WebGL tests, so you should always run tests locally as well. (Assuming they are failing for everyone and not just me)

@mramato
Copy link
Contributor

mramato commented Aug 14, 2019

I actually can't reproduce the failures any more, I think I might have been missing that last commit? Either way this is good to go, @OmarShehata feel free to merge if you don't need a second opinion on the shaders.

@mramato
Copy link
Contributor

mramato commented Aug 16, 2019

@OmarShehata did you need to take another look at this or can we merge?

@OmarShehata
Copy link
Contributor

Sorry for the delay here. This looks good to me.

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.

5 participants