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

Support programatic zoom #62

Merged
merged 11 commits into from
Jun 5, 2023
Merged

Support programatic zoom #62

merged 11 commits into from
Jun 5, 2023

Conversation

masato1230
Copy link
Contributor

Description

Add two functions that allow zooming to be executed programmatically.

  • zoomToOnContentCoordinate
    • zoomTo the position based on content size coordinate. This function can be used to implement the initial zoom functionality.
  • zoomToOnLayoutCoordinate
    • zoom to the position based on layout size coordinate. This function can be used to programatic zoom with some user gestures.

Sample(Capture)

  • initial zoom(0~2s)
  • zoom with long press(5s~)
device-2023-05-25-002658.mp4

Sample code used in video

@Composable
fun AsyncImageSample() {
    val zoomState = rememberZoomState()
    val coroutineScope = rememberCoroutineScope()
    AsyncImage(
        model = "https://github.com/usuiat.png",
        contentDescription = "GitHub icon",
        contentScale = ContentScale.Fit,
        onSuccess = { state ->
            zoomState.setContentSize(state.painter.intrinsicSize)
            coroutineScope.launch {
                zoomState.zoomToOnContentCoordinate(zoomTo = Offset(x = 230f, y = 203f))
            }
        },
        modifier = Modifier
            .fillMaxSize()
            .zoomable(zoomState)
            .pointerInput(1) {
                detectTapGestures(
                    onLongPress = {
                        coroutineScope.launch {
                            zoomState.zoomToOnLayoutCoordinate(it)
                        }
                    }
                )
            }
    )
}

@usuiat
Copy link
Owner

usuiat commented May 27, 2023

Thank you for submitting the PR!
I think it's a good idea to support programatic zoom.
I will review.

@usuiat
Copy link
Owner

usuiat commented May 28, 2023

The Image should be zoomed around a specified point, but this is not currently the case.

I will explain the case of zoomToOnContentCoordinate using a sample image.
The image is 512x512 pixel. If I specify Offset(128, 256), the specified point should not move on the screen. But actually it moves.

Original size Expected Actual
image image image

The same problem exists with zoomToOnLayoutCoordinate.

@usuiat
Copy link
Owner

usuiat commented May 28, 2023

I have just merged another branch to main branch.

In it, there is an internal function called changeScale.
Since changeScale is almost same to zoomToOnLayoutCoordinate, I would like to make this function public and rename it zoomToOnLayoutCoordinate.

And I think zoomToOnContentCoordinate can be achieved by transforming the coordinates and calling zoomToOnLayoutCoordinate.

@usuiat
Copy link
Owner

usuiat commented May 28, 2023

Sorry for the consecutive posts.
Did you implement it so that the specified point will be centered on the screen?
If so, I was misunderstood.

@masato1230
Copy link
Contributor Author

masato1230 commented May 29, 2023

@usuiat

Did you implement it so that the specified point will be centered on the screen?

Yes! I have created this Pull Request with the intention of implementing the functionality to center a specified point on the screen.

scale: Float = 3f,
animationSpec: AnimationSpec<Float> = tween(700),
) = coroutineScope {
val fitContentSizeFactor = fitContentSize.width / contentSize.height
Copy link
Owner

Choose a reason for hiding this comment

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

@masato1230
fitContentSizeFactor is calculated in width / height, but not width / width?

Suggested change
val fitContentSizeFactor = fitContentSize.width / contentSize.height
val fitContentSizeFactor = fitContentSize.width / contentSize.width

@usuiat
Copy link
Owner

usuiat commented Jun 2, 2023

@masato1230
I have two requests so that the library users will understand that the specified coordinates move to the center of the screen.

  • Change the function name. For example, how about centerByContentCoordinate(offset, scale, animationSpec) and centerByLayoutCoordinate(offset, scale, animationSpec)?
  • Add description to Kdoc of the functions.

@masato1230 masato1230 requested a review from usuiat June 4, 2023 04:28
@masato1230
Copy link
Contributor Author

@usuiat
Thank you for your review.
I've made the revisions you requested. Could you please review it again?

Copy link
Owner

@usuiat usuiat left a comment

Choose a reason for hiding this comment

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

LGTM👍 Thanks.

@usuiat usuiat merged commit 83a2784 into usuiat:main Jun 5, 2023
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.

2 participants