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

Update to use GeoviewCompose - SceneView #191

Merged
merged 9 commits into from
Apr 1, 2024

Conversation

shubham7109
Copy link
Collaborator

@shubham7109 shubham7109 commented Apr 1, 2024

Description

PR to update the 3 missed SceneView compose samples to use geoview-compose SceneView:

  • ShowViewshedFromPointInScene
  • AddSceneLayerWithElevation
  • DisplaySceneFromMobileScenePackage

Links and Data

Sample issue: runtime/kotlin/issues/3776

@shubham7109 shubham7109 self-assigned this Apr 1, 2024
Copy link

@ruiqima ruiqima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. Samples run well.

@shubham7109
Copy link
Collaborator Author

Thanks, @ruiqima, I addressed the feedback. Ready for your review

@shubham7109 shubham7109 requested a review from ruiqima April 1, 2024 18:29
Copy link

@ruiqima ruiqima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@prupani-7 prupani-7 self-requested a review April 1, 2024 19:28
Copy link

@prupani-7 prupani-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shubham7109 Looks good, one minor comment about the extra line.

Other comment:

  • For theshow-viewshed-from-point-in-scenesample, I noticed a typo in intialSliderValue in the ViewshedSlider.kt file, should be initialSliderValue
    since we are making changes to this file, it will be good to address this minor typo as well.

  • In the same file, (ignore this comment, I see its been addressed here)

var sliderValue by remember {
        mutableStateOf(intialSliderValue)
}

could be modified to

var sliderValue by remember {
        mutableFloatStateOf(intialSliderValue)
}

Copy link

@prupani-7 prupani-7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@shubham7109 shubham7109 merged commit c538ba4 into v.next Apr 1, 2024
1 check passed
@shubham7109 shubham7109 deleted the feature-branch/geoview-compose branch April 1, 2024 19:57
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.

3 participants