Fix Star Flickering by Positioning on Unit Sphere #283
Conversation
Removed `SCALING_FACTOR` in `CelestialObjectsUtils` to make the stars render on the unit sphere. This update ensures that the stars render correctly as the 100 scale factor was making the stars tilting cause the max view distance is 100 (see `Camera.kt`).
…ed imports Renamed package `utils` to `util` for `CelestialObjectsUtils` to maintain consistency. Updated imports across the project to reflect the new package structure. Moved `StarDataRepositoryTest` to align with the updated `map/stars` directory structure. Removed unused field-of-view constants from `MapViewModel.kt`. Adjusted formatting and fixed imports in `MapRenderer`, `Moon`, `PlanetsRepository`, and `StarsLoader`. Cleaned up redundant dependencies and ensured consistency in utility usage. This refactor improves code organization, readability, and adheres to updated package naming conventions.
Updated `CelestialObjectsUtilTest.kt` to adapt to the previous refactoring commit. Added `TODO` comments for deprecated or unused test functions (`convertToHorizonCoordinates`).
Updated `Moon.kt` to include a scale of `0.05f` for rendering. Modified `Planet.kt` to change the default scale from `0.3f` to `0.02f`. These changes ensure consistent rendering proportions across celestial objects since we changed the projection on a unit sphere.
# Conflicts: # app/src/main/java/com/github/lookupgroup27/lookup/model/map/stars/StarsLoader.kt
Annotated `getMoonPhaseTextureId` in `Moon.kt` with `@VisibleForTesting` to enable testing. Added a new test class `MoonTest` to validate moon phase texture ID mapping for various dates. Verified results against expected drawable resources for moon phases.
Moved `StarsLoader` to the correct package `com.github.lookupgroup27.lookup.model.map.stars`. Removed unnecessary import of `StarDataRepository`. Simplified the creation of `Star` objects by removing the redundant `color` parameter. Affected files: - `StarsLoader.kt`: Adjusted package declaration, imports, and star creation logic.
|
|
Improving the code coverage in this area is highly ineffective due to the need for a full mock of a GLSurfaceView, which introduces significant overhead for testing the Moon attributes. These attributes are trivially initialized, and the effort required to cover this code does not justify the complexity it adds. As a result, we will skip the SonarCloud analysis for this specific test to maintain efficiency and focus on more impactful improvements. |
There was a problem hiding this comment.
Great job addressing this issue. The changes are well done, and it's good to see the testable features were covered.
While the new code coverage falls short of passing the quality gate, this is primarily because the affected classes rely on a non-mockable OpenGL environment, making comprehensive testing challenging.
Given the limitations of the testing environment and the correctness of the implementation, I approve this pull request for merging.
Description
This pull request resolves the issue of stars flickering (disappearing and reappearing) when rendered at the middle of the screen. The problem was caused by stars being positioned on a sphere with a radius of 100, which matches the camera's maximum view distance. This update repositions all stars onto a unit sphere (radius of 1.0) to ensure consistent rendering.
Changes
CelestialObjectsUtils.kt:SCALING_FACTORto calculate positions on a unit sphere instead of a sphere with a radius of 100.CelestialObjectsUtilTest.ktto align with the new unit sphere logic.Testing