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

Added pixelRatio parameter to camera and the frustum objects #8237

Merged
merged 6 commits into from
Oct 24, 2019

Conversation

IanLilleyT
Copy link
Contributor

@IanLilleyT IanLilleyT commented Oct 1, 2019

Another CSS pixel fix for #8113

All frustums have a getPixelDimensions function that didn't account for different pixel density displays. So this PR adds a new pixelRatio parameter to these functions. Most systems should pass in frameState.pixelRatio to get a value in css pixels. This fixes a few different files that use minimum/maximum pixel size, including BillboardCollection, Model, Primitive, and PointPrimtiveCollection. This before and after are particularly noticeable on high dpi displays. Toggle the resolution checkbox. In the new version the model stays the same size.

Note: Picking.getPickOrthographicCullingVolume and Picking.getPickPerspectiveCullingVolume pass in 1.0 instead of scene.pixelRatio because picking wants to select from a physically smaller region on higher density screens (aka work with native device pixels instead of css pixels)

@cesium-concierge
Copy link

Thanks for the pull request @IanLilleyT!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

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

@lilleyse
Copy link
Contributor

Before merging this PR make sure to add all the fixed CSS pixels bugs in CHANGES.md

@IanLilleyT
Copy link
Contributor Author

@lilleyse I made some changes. Now camera uses its own scene's pixel ratio, so the Camera.getPixelSize function signature is the same as pre-PR. Anything that needs the more custom behavior can use camera.frustum.getPixelDimensions and pass in its own pixel ratio.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 22, 2019

@IanLilleyT @lilleyse what's the status of this? Are we trying to get this in for the release?

@lilleyse
Copy link
Contributor

Yes the plan is to get into the release - I'm good with everything here just need to do some final testing and review. Thanks for the reminder @hpinkos.

@lilleyse
Copy link
Contributor

@IanLilleyT looks good. Write an issue to remove the deprecated function definitions and add the 1.65 label. See #8183 for an example.

@lilleyse lilleyse merged commit 8cc27c8 into master Oct 24, 2019
@lilleyse lilleyse deleted the pixelRatioFrustum branch October 24, 2019 13:20
@IanLilleyT
Copy link
Contributor Author

Here's the issue for the deprecation: #8320

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

4 participants