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

Better zoom gestures #65

Closed
IacobIonut01 opened this issue Feb 10, 2024 · 13 comments
Closed

Better zoom gestures #65

IacobIonut01 opened this issue Feb 10, 2024 · 13 comments

Comments

@IacobIonut01
Copy link

I've tried your libray for subsampling in my Gallery app and I've noticed that the zoomable gesture handler is not as good as the previous solution I've used (https://github.com/usuiat/Zoomable) specially when used with lots of gesture (swipe up vertically, horizontal pager scroll etc)

Would be great if you can look into that library or allow the use of it because it would make the solution much better for Gallery/Media Viewer usescases

For Horizontal Pager even with the scroll disabled, it still conflicts with the other gesutres (swipe up)

LaunchedEffect(zoomableState.zoomFraction) {
    scrollEnabled.value = (zoomableState.zoomFraction ?: 0f) == 0.0f
}

Thanks!

@saket
Copy link
Owner

saket commented Feb 10, 2024

I've noticed that the zoomable gesture handler is not as good as the previous solution I've used (https://github.com/usuiat/Zoomable) specially when used with lots of gesture (swipe up vertically, horizontal pager scroll etc)

Can you share a video of the issues you're seeing? It's difficult to visualize otherwise.

For Horizontal Pager even with the scroll disabled, it still conflicts with the other gesutres (swipe up)

Same question. I'm not sure what's conflicting and a video will help.

@IacobIonut01
Copy link
Author

screen-20240210-201738.mp4

@IacobIonut01
Copy link
Author

ZoomableImage(
    modifier = modifier.fillMaxSize()
        .combinedClickable(
                  interactionSource = remember { MutableInteractionSource() },
                  indication = null,
                  onDoubleClick = {},
                  onClick = onItemClick
        ),
    state = state,
    image = ZoomableImageSource.coil3(
          model = ImageRequest.Builder(LocalPlatformContext.current)
                      .data(media.uri)
                      .placeholderMemoryCacheKey(media.toString())
                      .build()
    ),
    contentScale = ContentScale.Fit,
    contentDescription = media.label
)

@IacobIonut01
Copy link
Author

image

@saket
Copy link
Owner

saket commented Feb 11, 2024

The issue happening at 0:05 in the video looks strange. I wonder if it's happening because the image hasn't fully loaded yet. Are you using placeholder images? If so, can you add a loading indicator to verify if the gestures start working only after the image has fully loaded?

Btw, it'd be best if you can share a reproducer for me to investigate instead of going back and forth with questions. Your app looks very cool, I'd love to support it.

@IacobIonut01
Copy link
Author

Here is the app with the subsampling image applied (I've also made a custom source for coil3 (alpha)

For placeholder I'm using the coil's prebuilt memory cache key to get the lower quality image that was previously loaded in a grid view

https://github.com/IacobIonut01/Gallery/tree/0c3fac0b3246f5d82fb4b77ce54d267f2ac7d701

saket added a commit that referenced this issue Feb 14, 2024
@saket
Copy link
Owner

saket commented Feb 17, 2024

@IacobIonut01 I haven't gotten a chance to check out your project yet, but can you try out 0.8.0-SNAPSHOT? I made a small improvement to consume gestures immediately if multiple fingers/pointers are detected in 4c9118e.

@IacobIonut01
Copy link
Author

Sure, but I couldn't find it on maven

@saket
Copy link
Owner

saket commented Feb 17, 2024

You'll need to add the snapshot repository to your project for accessing snapshot versions:

maven {
  url 'https://oss.sonatype.org/content/repositories/snapshots/'
}

@IacobIonut01
Copy link
Author

Yeah it's better now

@saket
Copy link
Owner

saket commented Feb 17, 2024

Glad to hear! Are there any remaining improvements you need?

@IacobIonut01
Copy link
Author

Glad to hear! Are there any remaining improvements you need?

The only feature that is missing is progressive zoom (double tap 2x, another double tap 5x, another double tap max zoom for example)

@saket
Copy link
Owner

saket commented Feb 18, 2024

Cool, tracking that in #32

@saket saket closed this as completed Feb 18, 2024
github-merge-queue bot pushed a commit to slackhq/circuit that referenced this issue Feb 18, 2024
…1215)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[me.saket.telephoto:zoomable-image-coil](https://togithub.com/saket/telephoto)
| dependencies | minor | `0.7.1` -> `0.8.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>saket/telephoto
(me.saket.telephoto:zoomable-image-coil)</summary>

### [`v0.8.0`](https://togithub.com/saket/telephoto/releases/tag/0.8.0)

[Compare
Source](https://togithub.com/saket/telephoto/compare/0.7.1...0.8.0)

Breaking changes

- Reordered `SubSamplingImage()`'s parameters to move `Modifier` below
all required parameters.

New changes

- Update Compose UI to `1.6.1` and Compose Multiplatform to `1.6.0-rc02`
- Introduced
[ZoomableState#transformedContentBounds](https://togithub.com/saket/telephoto/blob/706cf08cb976c0d9d9c6d0f95e4e64fc4efbf4ef/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/ZoomableState.kt#L88-L96)
for observing transformed content bounds. This can be used for drawing
decorations around the content or performing hit tests.
- Placeholder images now respond to click listeners. Additionally, they
will swallow all other zoom gestures instead of ignoring them.
-
[saket/telephoto#3:
Read color space of bitmaps from Coil and Glide.

Bug fixes

-
[saket/telephoto#60,
[saket/telephoto#65:
Improved detection of pinch-to-zoom gestures.
-
[saket/telephoto#8:
Composables with `Modifier.zoomable()` are now drawn on the first frame.
This fixes their broken layout preview.
-
[saket/telephoto#58:
Fixed a resource leak when an image's EXIF metadata is read.
- `ZoomableState#resetZoom()` now calculates the content's position on
the same UI frame.
- Images no longer flicker on start when they can't zoom-in any further.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xOTQuMyIsInVwZGF0ZWRJblZlciI6IjM3LjE5NC4zIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
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

No branches or pull requests

2 participants