From 11591826f0093515d113ea5bce4b5ab11372a224 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 15:37:44 +0000 Subject: [PATCH 1/8] fix: detach onchain-zap and nutzap sources when pruning Notes from LocalCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LocalCache must hold a single Note per event id/address and must never remove a Note from the cache map while another Note still strongly references it — a dangling reference both leaks the shell and lets a relay echo mint a second Note with the same id. Note.onchainZaps (NIP-BC) and Note.nutzaps (NIP-61) were added after the removal/migration routines were written and were never wired into them, so a pruned zap-source Note leaked through its target's maps: - removeNote() only detached reply/boost/reaction/zap/zapPayment/report/ label, leaving the target's onchainZaps/nutzaps entry dangling when the source note was pruned. Now also calls removeNutzap + a new source-keyed removeOnchainZap (unconditional cache removal, distinct from the verdict-respecting removeOnchainZapForSource). - removeAllChildNotes() cleared onchainZaps but never returned the source notes for removal from the cache map (asymmetric with nutzaps), so they lingered orphaned. Now included. - moveAllReferencesTo() dropped labels, zapPayments, and onchainZaps when a replaceable's old version was superseded — silent data loss plus orphaned onchain sources. Now migrated and cleared like the rest. Adds NotePruningReferenceTest covering all three paths. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../amethyst/commons/model/Note.kt | 46 ++++- .../commons/model/NotePruningReferenceTest.kt | 165 ++++++++++++++++++ 2 files changed, 210 insertions(+), 1 deletion(-) create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index ebe9e27d02..c3f469f1c3 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -146,6 +146,8 @@ open class Note( removeZapPayment(note) removeReport(note) removeLabel(note) + removeNutzap(note) + removeOnchainZap(note) } var poll: PollResponsesCache? = null @@ -389,7 +391,8 @@ open class Note( zaps.values.filterNotNull() + zapPayments.keys + zapPayments.values.filterNotNull() + - nutzaps.values.map { it.source } + nutzaps.values.map { it.source } + + onchainZaps.values.map { it.source } replies = listOf() reactions = mapOf() @@ -587,6 +590,29 @@ open class Note( } } + private fun innerRemoveOnchainZapBySource(source: Note): Boolean = + syncLock.withLock { + val newMap = onchainZaps.filterValues { it.source != source } + if (newMap.size == onchainZaps.size) return@withLock false + onchainZaps = newMap + return@withLock true + } + + /** + * Detach every onchain-zap entry whose source is [source] — used when the + * source OnchainZapEvent note is being pruned from `LocalCache`. Unlike + * [removeOnchainZapForSource] (a verification verdict that respects the + * anti-spoof / no-CONFIRMED-downgrade guards), this is an unconditional + * cache-removal that must drop the strong reference no matter the status, + * otherwise the pruned source Note leaks through this map. + */ + fun removeOnchainZap(source: Note) { + if (innerRemoveOnchainZapBySource(source)) { + updateZapTotal() + flowSet?.zaps?.invalidateData() + } + } + private fun innerAddNutzap( eventId: HexKey, entry: NutzapEntry, @@ -1165,6 +1191,21 @@ open class Note( note.addNutzap(it.source, it.claimedSats) it.source.replyTo = it.source.replyTo?.replace(this, note) } + onchainZaps.forEach { (txid, entry) -> + note.addOnchainZap(entry.source, txid, entry.claimedSats, entry.verifiedSats, entry.status) + entry.source.replyTo = entry.source.replyTo?.replace(this, note) + } + zapPayments.forEach { + note.addZapPayment(it.key, it.value) + it.key.replyTo = it.key.replyTo?.replace(this, note) + it.value?.replyTo = it.value?.replyTo?.replace(this, note) + } + labels.forEach { (hashtag, labelNotes) -> + labelNotes.forEach { + note.addLabel(hashtag, it) + it.replyTo = it.replyTo?.replace(this, note) + } + } replyTo = null replies = emptyList() @@ -1173,6 +1214,9 @@ open class Note( reports = emptyMap() zaps = emptyMap() nutzaps = emptyMap() + onchainZaps = emptyMap() + zapPayments = emptyMap() + labels = emptyMap() zapsAmount = BigDecimal(0) } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt new file mode 100644 index 0000000000..1b065ac826 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt @@ -0,0 +1,165 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.model + +import com.vitorpamplona.quartz.nip01Core.core.Address +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * Guards the cache-pruning invariant: when a Note is removed from `LocalCache`, + * every other Note that referenced it must drop that strong reference. Onchain + * zaps (NIP-BC) and nutzaps (NIP-61) were added to [Note] after the original + * removal/migration routines were written, so these tests pin that + * [Note.removeNote], [Note.removeAllChildNotes], and [Note.moveAllReferencesTo] + * all account for them — otherwise a pruned source Note leaks through the + * target's `onchainZaps` / `nutzaps` map and a duplicate Note with the same id + * gets minted on the next relay echo. + */ +class NotePruningReferenceTest { + private fun note(idHex: String) = Note(idHex) + + private fun userFor(pubKey: HexKey) = User(pubKey) { addr -> Note(addr.toValue()) } + + // A source-event Note with a wired author, matching the production shape where + // `source.author` is the zap sender. + private fun sourceNote(pubKey: HexKey): Note = note(pubKey).apply { author = userFor(pubKey) } + + private fun eventWith(idHex: HexKey): Event = + Event( + id = idHex, + pubKey = "ab".repeat(32), + createdAt = 1L, + kind = 9321, + tags = emptyArray(), + content = "", + sig = "sig", + ) + + // ── Fix 1: removeNote drops onchain-zap and nutzap sources ────────────── + + @Test + fun removeNoteDetachesOnchainZapSource() { + val target = note("a".repeat(64)) + val src = sourceNote("11".repeat(32)) + + target.addOnchainZap(src, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + assertTrue(target.onchainZaps.containsKey("tx1")) + + target.removeNote(src) + + assertTrue(target.onchainZaps.isEmpty(), "onchain zap source must be detached on removeNote") + } + + @Test + fun removeNoteDetachesNutzapSource() { + val target = note("b".repeat(64)) + val src = sourceNote("22".repeat(32)).apply { event = eventWith("cc".repeat(32)) } + + target.addNutzap(src, claimedSats = 500L) + assertTrue(target.nutzaps.isNotEmpty()) + + target.removeNote(src) + + assertTrue(target.nutzaps.isEmpty(), "nutzap source must be detached on removeNote") + } + + @Test + fun removeOnchainZapBySourceIgnoresUnrelatedSource() { + val target = note("d".repeat(64)) + val src = sourceNote("33".repeat(32)) + val other = sourceNote("44".repeat(32)) + + target.addOnchainZap(src, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + + target.removeNote(other) + + assertTrue(target.onchainZaps.containsKey("tx1"), "removing an unrelated note must not drop the entry") + } + + // ── Fix 2: removeAllChildNotes returns onchain-zap sources ────────────── + + @Test + fun removeAllChildNotesReturnsAndClearsOnchainZapSources() { + val target = note("e".repeat(64)) + val src = sourceNote("55".repeat(32)) + + target.addOnchainZap(src, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + + val removed = target.removeAllChildNotes() + + assertTrue(src in removed, "onchain zap source must be returned for removal from the cache map") + assertTrue(target.onchainZaps.isEmpty()) + } + + // ── Fix 3: moveAllReferencesTo migrates onchain zaps, zap payments, labels ── + + @Test + fun moveAllReferencesToMigratesOnchainZaps() { + val old = note("f0".repeat(32)) + val newer = AddressableNote(Address(30023, "ab".repeat(32), "slug")) + val src = sourceNote("66".repeat(32)).apply { replyTo = listOf(old) } + + old.addOnchainZap(src, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + + old.moveAllReferencesTo(newer) + + assertTrue(old.onchainZaps.isEmpty(), "old version must release its onchain zaps") + assertEquals(1, newer.onchainZaps.size, "onchain zap must move to the newer version") + assertSame(src, newer.onchainZaps["tx1"]?.source) + assertSame(newer, src.replyTo?.single(), "source replyTo must repoint to the newer version") + } + + @Test + fun moveAllReferencesToMigratesZapPayments() { + val old = note("f1".repeat(32)) + val newer = AddressableNote(Address(30023, "ab".repeat(32), "slug2")) + val request = note("77".repeat(32)).apply { replyTo = listOf(old) } + val response = note("88".repeat(32)) + + old.addZapPayment(request, response) + + old.moveAllReferencesTo(newer) + + assertTrue(old.zapPayments.isEmpty(), "old version must release its zap payments") + assertTrue(newer.zapPayments.containsKey(request), "zap payment must move to the newer version") + assertSame(newer, request.replyTo?.single()) + } + + @Test + fun moveAllReferencesToMigratesLabels() { + val old = note("f2".repeat(32)) + val newer = AddressableNote(Address(30023, "ab".repeat(32), "slug3")) + val labelNote = note("99".repeat(32)).apply { replyTo = listOf(old) } + + old.addLabel("nostr", labelNote) + + old.moveAllReferencesTo(newer) + + assertTrue(old.labels.isEmpty(), "old version must release its labels") + assertTrue(newer.labels["nostr"]?.contains(labelNote) == true, "label must move to the newer version") + assertSame(newer, labelNote.replyTo?.single()) + } +} From c4bfdc72dc0a4fcff1648c87eef7a58888935fa4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 16:17:14 +0000 Subject: [PATCH 2/8] fix: detach channel in removeFromCache to mirror deleteNote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit removeFromCache() relied solely on note.inGatherers to detach a pruned note from its channels, while deleteNote() additionally resolved the channel via getAnyChannel() and removed the note there too. inGatherers is normally authoritative (Channel.addNote always calls addGatherer), so this is defensive rather than a confirmed live leak — but it closes the divergence so both removal paths detach channels identically. Guards against any future consume path that adds a note to a getAnyChannel- resolvable channel without the gatherer link: otherwise the note would linger in the channel's notes map after leaving the cache, leaking it and letting a relay echo mint a duplicate Note with the same id. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../java/com/vitorpamplona/amethyst/model/LocalCache.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index f7847f225a..c9372f62d4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -2785,6 +2785,13 @@ object LocalCache : ILocalCache, ICacheProvider { note.inGatherers?.forEach { it.removeNote(note) } + // Mirror deleteNote(): inGatherers is normally authoritative for channel + // membership (Channel.addNote always calls note.addGatherer), but resolve + // the channel from the event as a belt-and-suspenders detach so a note can + // never linger in a channel's notes map after it leaves the cache — that + // would leak the note and let a relay echo mint a duplicate with the same id. + getAnyChannel(note)?.removeNote(note) + val noteEvent = note.event if (noteEvent is ReportEvent) { From 23ddeba8ec2038304d0ee29959402d0a236a8940 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 17:42:29 +0000 Subject: [PATCH 3/8] fix: sever child back-references when deleting a Note (NIP-09) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleteNote() removed the target from its parents, gatherers, and the cache map, but never cleared its own child collections nor dropped itself from its children's replyTo. That left a partial deletion: every child kept the removed shell alive through replyTo (a leak), and a reply resolved later via computeReplyTo would getOrCreateNote a *second* Note for the same id — breaking the one-Note-per-id invariant. Adds Note.detachFromChildren(), which clears the note's forward child collections (via removeAllChildNotes) and severs this note from each child's replyTo (keeping any other parents). deleteNote() now calls it before notes.remove(), so once the note leaves the map nothing points at the dead shell. Orphaned replies become roots, which is correct once their parent is hard-deleted from the cache. Adds detachFromChildren coverage to NotePruningReferenceTest. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../amethyst/model/LocalCache.kt | 6 +++ .../amethyst/commons/model/Note.kt | 25 ++++++++++ .../commons/model/NotePruningReferenceTest.kt | 48 +++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index c9372f62d4..878b7fa0ca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -1379,6 +1379,12 @@ object LocalCache : ILocalCache, ICacheProvider { getAnyChannel(deleteNote)?.removeNote(deleteNote) + // Sever the back-references from this note's children before it leaves the + // map. Otherwise each child keeps the removed shell alive through its replyTo + // (a partial deletion / leak) and a reply resolved later via computeReplyTo + // would resurrect a second Note for the same id. + deleteNote.detachFromChildren() + notes.remove(deleteNote.idHex) deleteNote.clearFlow() diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index c3f469f1c3..f1bbaaeb7c 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -417,6 +417,31 @@ open class Note( return toBeRemoved } + /** + * Fully detach this note from the notes below it in the graph so it can be + * removed from the cache without leaving a partial deletion behind. It both + * clears this note's own child collections (via [removeAllChildNotes]) and + * drops this note from every child's [replyTo], so once this note leaves the + * cache map nothing keeps the dead shell alive. + * + * This matters for the NIP-09 delete path: without severing the child → + * parent `replyTo` links, the removed note leaks (held by each child) and a + * later reply resolved through `computeReplyTo` would `getOrCreateNote` a + * *second* Note for the same id — breaking the one-Note-per-id invariant. + * + * Returns the now-orphaned children (their other parents, if any, are kept). + */ + fun detachFromChildren(): List { + val children = removeAllChildNotes() + children.forEach { child -> + val parents = child.replyTo + if (parents != null && this in parents) { + child.replyTo = parents - this + } + } + return children + } + fun removeReaction(note: Note) { val tags = note.event?.tags ?: emptyArray() val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt index 1b065ac826..6618e33716 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt @@ -162,4 +162,52 @@ class NotePruningReferenceTest { assertTrue(newer.labels["nostr"]?.contains(labelNote) == true, "label must move to the newer version") assertSame(newer, labelNote.replyTo?.single()) } + + // ── Fix 4: deleteNote severs child back-references (no partial deletion) ── + + @Test + fun detachFromChildrenSeversReplyToAndClearsCollections() { + val parent = note("a1".repeat(32)) + val reply = note("b1".repeat(32)).apply { replyTo = listOf(parent) } + parent.addReply(reply) + + val detached = parent.detachFromChildren() + + assertTrue(reply in detached, "the child must be returned as detached") + assertTrue(parent.replies.isEmpty(), "parent must release its forward child references") + assertTrue( + reply.replyTo?.contains(parent) != true, + "child must no longer point back at the removed parent", + ) + } + + @Test + fun detachFromChildrenKeepsOtherParents() { + val deleted = note("a2".repeat(32)) + val survivor = note("c2".repeat(32)) + val reply = note("b2".repeat(32)).apply { replyTo = listOf(deleted, survivor) } + deleted.addReply(reply) + survivor.addReply(reply) + + deleted.detachFromChildren() + + assertEquals(listOf(survivor), reply.replyTo, "only the removed parent must be dropped from replyTo") + } + + @Test + fun detachFromChildrenSeversReactionAndZapSources() { + val parent = note("a3".repeat(32)) + val reaction = sourceNote("31".repeat(32)).apply { replyTo = listOf(parent) } + val zapSource = sourceNote("32".repeat(32)).apply { replyTo = listOf(parent) } + + parent.addOnchainZap(zapSource, "tx1", claimedSats = 1L, verifiedSats = 1L, status = OnchainZapStatus.CONFIRMED) + parent.addBoost(reaction) + + parent.detachFromChildren() + + assertTrue(parent.boosts.isEmpty()) + assertTrue(parent.onchainZaps.isEmpty()) + assertTrue(reaction.replyTo?.contains(parent) != true) + assertTrue(zapSource.replyTo?.contains(parent) != true) + } } From a3bb6fbc100b13329789c595df8ecd1ed72d70c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 18:35:56 +0000 Subject: [PATCH 4/8] refactor: unify Note removal into one shared unlink path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleteNote() (NIP-09) and removeFromCache() (prune) had drifted into two near-duplicate "detach a note from the cache" routines. Removal really has two halves — (1) unlink the note from everything that points AT it, and (2) handle the note's OWN children — and only the second half differs between the paths. removeFromCache() already implemented half (1) completely, so deleteNote() now delegates to it and keeps only its two delete-specific responsibilities: tearing down gift-wrap hosts and severing (but keeping) its children via detachFromChildren(). This also fixes a real leak the duplication was hiding. computeReplyTo() has no ReportEvent branch, so a report note's replyTo is empty and the report→target link lives only in the explicit reported* index handling. The old deleteNote() only undid reportedAuthor(), so deleting an event-level report (reportedPost / reportedAddresses) left the reported note's `.reports` map holding the removed report note — a partial deletion that leaked the shell and risked a duplicate Note for the same id. Delegating to removeFromCache() (author + post + addresses, all idempotent) closes that gap. Net behavior change is the report-leak fix only; the redundant TorrentCommentEvent case is dropped because the torrent target is already in replyTo (and removed via removeNote), and its @Suppress("DEPRECATION") goes with it. Adds KDoc to both methods documenting the two-halves model. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../amethyst/model/LocalCache.kt | 95 ++++++++----------- 1 file changed, 40 insertions(+), 55 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 878b7fa0ca..0c101d7b4f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -1336,60 +1336,28 @@ object LocalCache : ILocalCache, ICacheProvider { else -> null } - @Suppress("DEPRECATION") + /** + * NIP-09 delete of a single targeted event. + * + * Removal has two halves: unlinking the note from everything that points AT it + * (its parents, channels, and the per-user report/card/status/poll indexes — + * all handled by [removeFromCache]); and dealing with the note's OWN children + * (the notes that point at IT). The delete path and the prune path share the + * first half and differ only on the second: + * - delete (here): the children are independent events and stay in the cache; + * [Note.detachFromChildren] only severs their back-reference so the removed + * shell can neither leak (held alive by a child's `replyTo`) nor be later + * resurrected by `computeReplyTo` as a second Note for the same id. + * - prune (see [removeFromCache] callers): the whole child subtree is removed. + * + * Gift-wrapped events additionally drop their decrypted inner host. + */ private fun deleteNote(deleteNote: Note) { - val deletedEvent = deleteNote.event + (deleteNote.event as? WrappedEvent)?.let { deleteWraps(it) } - if (deletedEvent is ReportEvent) { - deletedEvent.reportedAuthor().forEach { - getUserIfExists(it.pubkey)?.reportsOrNull()?.removeReport(deleteNote) - } - } - - if (deleteNote is AddressableNote && deletedEvent is ContactCardEvent) { - getUserIfExists(deletedEvent.aboutUser())?.cardsOrNull()?.removeCard(deleteNote) - } - - if (deleteNote is AddressableNote && deletedEvent is StatusEvent) { - deleteNote.author?.statusStateOrNull()?.removeStatus(deleteNote) - } - - if (deletedEvent is PollResponseEvent) { - deletedEvent.poll()?.eventId?.let { - getNoteIfExists(it)?.pollStateOrNull()?.removeResponse(deleteNote) - } - } - - if (deletedEvent is TorrentCommentEvent) { - deletedEvent.torrentIds()?.let { - getNoteIfExists(it)?.removeReply(deleteNote) - } - } - - if (deletedEvent is WrappedEvent) { - deleteWraps(deletedEvent) - } - - // Counts the replies - deleteNote.replyTo?.forEach { masterNote -> - masterNote.removeNote(deleteNote) - } - - deleteNote.inGatherers?.forEach { it.removeNote(deleteNote) } - - getAnyChannel(deleteNote)?.removeNote(deleteNote) - - // Sever the back-references from this note's children before it leaves the - // map. Otherwise each child keeps the removed shell alive through its replyTo - // (a partial deletion / leak) and a reply resolved later via computeReplyTo - // would resurrect a second Note for the same id. deleteNote.detachFromChildren() - notes.remove(deleteNote.idHex) - - deleteNote.clearFlow() - - refreshDeletedNoteObservers(deleteNote) + removeFromCache(deleteNote) } fun deleteWraps(event: WrappedEvent) { @@ -2784,6 +2752,28 @@ object LocalCache : ILocalCache, ICacheProvider { } } + /** + * Unlinks [note] from everything in the cache that references it, then drops it + * from the [notes] map and notifies observers. This is the shared "unlink from + * above" half of removal, used by both the prune callers and [deleteNote]. + * + * It detaches the note from: + * - its parent notes (their replies/reactions/zaps/boosts/reports/labels maps); + * because event-level reports and torrent comments both carry the target in + * `replyTo`, [Note.removeNote] cleans those up here too; + * - its channels/gatherers (`inGatherers` is authoritative — `Channel.addNote` + * always registers the gatherer — and `getAnyChannel` is a belt-and-suspenders + * resolve so a note can never linger in a channel after leaving the cache); + * - the per-target indexes `replyTo` does NOT reach: user-level reports and + * reported addresses, contact cards, statuses, and poll responses. + * + * It deliberately does NOT touch the note's own children: prune callers collect + * them via [Note.removeAllChildNotes] and remove the subtree, while [deleteNote] + * keeps them and severs only their back-reference. Every per-target removal is + * idempotent, so the overlap between `replyTo` and the explicit indexes (e.g. an + * event-level report reachable both ways) is harmless. Addressable notes are + * dropped from the [addressables] map by the caller; this only removes from [notes]. + */ private fun removeFromCache(note: Note) { note.replyTo?.forEach { masterNote -> masterNote.removeNote(note) @@ -2791,11 +2781,6 @@ object LocalCache : ILocalCache, ICacheProvider { note.inGatherers?.forEach { it.removeNote(note) } - // Mirror deleteNote(): inGatherers is normally authoritative for channel - // membership (Channel.addNote always calls note.addGatherer), but resolve - // the channel from the event as a belt-and-suspenders detach so a note can - // never linger in a channel's notes map after it leaves the cache — that - // would leak the note and let a relay echo mint a duplicate with the same id. getAnyChannel(note)?.removeNote(note) val noteEvent = note.event From 7c95ba1ffda6d3c00e02fd813f024456d6833998 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 18:59:54 +0000 Subject: [PATCH 5/8] refactor: rename removal methods to match what they do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three names didn't describe their behavior: - removeFromCache → unlinkAndRemove: the method's main job is unlinking the note from every referrer (parents, channels, the report/card/status/poll indexes), not just evicting it from the map; the old name only captured the last step. - removeAllChildNotes → clearChildLinks: it clears only THIS note's forward child collections and returns them — it does not touch the children's replyTo and does not remove anything from the cache. The old name sounded more aggressive than detachFromChildren(), which is actually the both-directions op. - Note.removeOnchainZap(source) → removeOnchainZapBySource(source): too easy to confuse with removeOnchainZapForSource(txid, pubkey), which is the verification-verdict removal with anti-spoof guards. The new name matches its inner helper (innerRemoveOnchainZapBySource) and disambiguates the two. Pure rename: no behavior change. Test names/comments updated to match. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../amethyst/model/LocalCache.kt | 64 +++++++++---------- .../amethyst/commons/model/Note.kt | 10 +-- .../commons/model/NoteOnchainZapTest.kt | 6 +- .../commons/model/NotePruningReferenceTest.kt | 8 +-- 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 0c101d7b4f..614c1edf75 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -1341,14 +1341,14 @@ object LocalCache : ILocalCache, ICacheProvider { * * Removal has two halves: unlinking the note from everything that points AT it * (its parents, channels, and the per-user report/card/status/poll indexes — - * all handled by [removeFromCache]); and dealing with the note's OWN children + * all handled by [unlinkAndRemove]); and dealing with the note's OWN children * (the notes that point at IT). The delete path and the prune path share the * first half and differ only on the second: * - delete (here): the children are independent events and stay in the cache; * [Note.detachFromChildren] only severs their back-reference so the removed * shell can neither leak (held alive by a child's `replyTo`) nor be later * resurrected by `computeReplyTo` as a second Note for the same id. - * - prune (see [removeFromCache] callers): the whole child subtree is removed. + * - prune (see [unlinkAndRemove] callers): the whole child subtree is removed. * * Gift-wrapped events additionally drop their decrypted inner host. */ @@ -1357,7 +1357,7 @@ object LocalCache : ILocalCache, ICacheProvider { deleteNote.detachFromChildren() - removeFromCache(deleteNote) + unlinkAndRemove(deleteNote) } fun deleteWraps(event: WrappedEvent) { @@ -2566,12 +2566,12 @@ object LocalCache : ILocalCache, ICacheProvider { val childrenToBeRemoved = mutableListOf() toBeRemoved.forEach { - removeFromCache(it) + unlinkAndRemove(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) if (toBeRemoved.size > 100 || channel.notes.size() > 100) { println( @@ -2605,12 +2605,12 @@ object LocalCache : ILocalCache, ICacheProvider { val childrenToBeRemoved = mutableListOf() toBeRemoved.forEach { - removeFromCache(it) + unlinkAndRemove(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) // Audio-room presence is keyed separately from `notes` and // never gets reaped by the top-N rule. Drop entries older @@ -2650,12 +2650,12 @@ object LocalCache : ILocalCache, ICacheProvider { toBeRemoved.forEach { childrenToBeRemoved.addAll(removeIfWrap(it)) - removeFromCache(it) + unlinkAndRemove(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) if (toBeRemoved.size > 1) { println( @@ -2673,8 +2673,8 @@ object LocalCache : ILocalCache, ICacheProvider { if (noteEvent is WrappedEvent) { noteEvent.host?.id?.let { getNoteIfExists(it)?.let { it2 -> - removeFromCache(it2) - it2.removeAllChildNotes() + unlinkAndRemove(it2) + it2.clearChildLinks() } } } else { @@ -2704,11 +2704,11 @@ object LocalCache : ILocalCache, ICacheProvider { it.moveAllReferencesTo(newerVersion) } - removeFromCache(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + unlinkAndRemove(it) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) if (toBeRemoved.size > 1) { println("PRUNE: ${toBeRemoved.size} old version of addressables removed.") @@ -2741,11 +2741,11 @@ object LocalCache : ILocalCache, ICacheProvider { val childrenToBeRemoved = mutableListOf() toBeRemoved.forEach { - removeFromCache(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + unlinkAndRemove(it) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) if (toBeRemoved.size > 1) { println("PRUNE: ${toBeRemoved.size} thread replies removed.") @@ -2768,13 +2768,13 @@ object LocalCache : ILocalCache, ICacheProvider { * reported addresses, contact cards, statuses, and poll responses. * * It deliberately does NOT touch the note's own children: prune callers collect - * them via [Note.removeAllChildNotes] and remove the subtree, while [deleteNote] + * them via [Note.clearChildLinks] and remove the subtree, while [deleteNote] * keeps them and severs only their back-reference. Every per-target removal is * idempotent, so the overlap between `replyTo` and the explicit indexes (e.g. an * event-level report reachable both ways) is harmless. Addressable notes are * dropped from the [addressables] map by the caller; this only removes from [notes]. */ - private fun removeFromCache(note: Note) { + private fun unlinkAndRemove(note: Note) { note.replyTo?.forEach { masterNote -> masterNote.removeNote(note) } @@ -2820,8 +2820,8 @@ object LocalCache : ILocalCache, ICacheProvider { refreshDeletedNoteObservers(note) } - fun removeFromCache(nextToBeRemoved: List) { - nextToBeRemoved.forEach { note -> removeFromCache(note) } + fun unlinkAndRemove(nextToBeRemoved: List) { + nextToBeRemoved.forEach { note -> unlinkAndRemove(note) } } fun pruneExpiredEvents() { @@ -2834,16 +2834,16 @@ object LocalCache : ILocalCache, ICacheProvider { val childrenToBeRemoved = mutableListOf() versionsToBeRemoved.forEach { - removeFromCache(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + unlinkAndRemove(it) + childrenToBeRemoved.addAll(it.clearChildLinks()) } addressesToBeRemoved.forEach { - removeFromCache(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + unlinkAndRemove(it) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) if (versionsToBeRemoved.size > 1 || addressesToBeRemoved.size > 1) { println("PRUNE: ${versionsToBeRemoved.size} events and ${addressesToBeRemoved.size} expired.") @@ -2861,11 +2861,11 @@ object LocalCache : ILocalCache, ICacheProvider { } toBeRemoved.forEach { - removeFromCache(it) - childrenToBeRemoved.addAll(it.removeAllChildNotes()) + unlinkAndRemove(it) + childrenToBeRemoved.addAll(it.clearChildLinks()) } - removeFromCache(childrenToBeRemoved) + unlinkAndRemove(childrenToBeRemoved) println("PRUNE: ${toBeRemoved.size} messages removed because they were Hidden") } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index f1bbaaeb7c..7c61246550 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -147,7 +147,7 @@ open class Note( removeReport(note) removeLabel(note) removeNutzap(note) - removeOnchainZap(note) + removeOnchainZapBySource(note) } var poll: PollResponsesCache? = null @@ -373,7 +373,7 @@ open class Note( } } - fun removeAllChildNotes(): List { + fun clearChildLinks(): List { val repliesChanged = replies.isNotEmpty() val reactionsChanged = reactions.isNotEmpty() val zapsChanged = zaps.isNotEmpty() || zapPayments.isNotEmpty() || onchainZaps.isNotEmpty() || nutzaps.isNotEmpty() @@ -420,7 +420,7 @@ open class Note( /** * Fully detach this note from the notes below it in the graph so it can be * removed from the cache without leaving a partial deletion behind. It both - * clears this note's own child collections (via [removeAllChildNotes]) and + * clears this note's own child collections (via [clearChildLinks]) and * drops this note from every child's [replyTo], so once this note leaves the * cache map nothing keeps the dead shell alive. * @@ -432,7 +432,7 @@ open class Note( * Returns the now-orphaned children (their other parents, if any, are kept). */ fun detachFromChildren(): List { - val children = removeAllChildNotes() + val children = clearChildLinks() children.forEach { child -> val parents = child.replyTo if (parents != null && this in parents) { @@ -631,7 +631,7 @@ open class Note( * cache-removal that must drop the strong reference no matter the status, * otherwise the pruned source Note leaks through this map. */ - fun removeOnchainZap(source: Note) { + fun removeOnchainZapBySource(source: Note) { if (innerRemoveOnchainZapBySource(source)) { updateZapTotal() flowSet?.zaps?.invalidateData() diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt index 4c91551d71..9587b5943c 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt @@ -349,15 +349,15 @@ class NoteOnchainZapTest { } @Test - fun removeAllChildNotesClearsOnchainZapResolvedFlag() { - // `removeAllChildNotes()` runs on delete-event handling and during cache + fun clearChildLinksResetsOnchainZapResolvedFlag() { + // `clearChildLinks()` runs on delete-event handling and during cache // pressure; the resolved flag must travel with the cleared state so a // re-arrival of the same event gets re-verified instead of being silently // skipped against stale state. val src = sourceNote("ee".repeat(32)) src.onchainZapResolved = true - src.removeAllChildNotes() + src.clearChildLinks() assertFalse(src.onchainZapResolved) } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt index 6618e33716..888957d103 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt @@ -33,7 +33,7 @@ import kotlin.test.assertTrue * every other Note that referenced it must drop that strong reference. Onchain * zaps (NIP-BC) and nutzaps (NIP-61) were added to [Note] after the original * removal/migration routines were written, so these tests pin that - * [Note.removeNote], [Note.removeAllChildNotes], and [Note.moveAllReferencesTo] + * [Note.removeNote], [Note.clearChildLinks], and [Note.moveAllReferencesTo] * all account for them — otherwise a pruned source Note leaks through the * target's `onchainZaps` / `nutzaps` map and a duplicate Note with the same id * gets minted on the next relay echo. @@ -99,16 +99,16 @@ class NotePruningReferenceTest { assertTrue(target.onchainZaps.containsKey("tx1"), "removing an unrelated note must not drop the entry") } - // ── Fix 2: removeAllChildNotes returns onchain-zap sources ────────────── + // ── Fix 2: clearChildLinks returns onchain-zap sources ────────────── @Test - fun removeAllChildNotesReturnsAndClearsOnchainZapSources() { + fun clearChildLinksReturnsAndClearsOnchainZapSources() { val target = note("e".repeat(64)) val src = sourceNote("55".repeat(32)) target.addOnchainZap(src, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) - val removed = target.removeAllChildNotes() + val removed = target.clearChildLinks() assertTrue(src in removed, "onchain zap source must be returned for removal from the cache map") assertTrue(target.onchainZaps.isEmpty()) From 5edfe90321bc1f2147f67eaff4b0530c617eb0d7 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 4 Jun 2026 19:34:58 +0200 Subject: [PATCH 6/8] feat(player): enable brightness/volume swipe in fullscreen video --- .../playback/composable/RenderVideoPlayer.kt | 40 ++- .../playback/composable/VideoViewInner.kt | 1 + .../controls/FullscreenSwipeControls.kt | 268 ++++++++++++++++++ .../controls/FullscreenSwipeMath.kt | 46 +++ .../controls/FullscreenSwipeMathTest.kt | 78 +++++ .../font/material_symbols_outlined.ttf | Bin 435080 -> 437648 bytes .../commons/icons/symbols/MaterialSymbols.kt | 1 + 7 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt index 77a390d21b..5c488fde80 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt @@ -20,11 +20,14 @@ */ package com.vitorpamplona.amethyst.service.playback.composable +import android.content.Context +import android.media.AudioManager import androidx.annotation.OptIn import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.MutableState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember @@ -34,18 +37,23 @@ import androidx.compose.ui.geometry.Size import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.platform.LocalContext import androidx.media3.common.Player import androidx.media3.common.util.UnstableApi import androidx.media3.ui.compose.ContentFrame import androidx.media3.ui.compose.SURFACE_TYPE_TEXTURE_VIEW import com.vitorpamplona.amethyst.service.playback.composable.controls.BottomGradientOverlay +import com.vitorpamplona.amethyst.service.playback.composable.controls.FullscreenSwipeControlsState +import com.vitorpamplona.amethyst.service.playback.composable.controls.FullscreenSwipeLevelIndicator import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderAnimatedBottomInfo import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderCenterButtons import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderTopButtons import com.vitorpamplona.amethyst.service.playback.composable.controls.TopGradientOverlay +import com.vitorpamplona.amethyst.service.playback.composable.controls.fullscreenSwipeControls import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.LoadedMediaItem import com.vitorpamplona.amethyst.service.playback.composable.wavefront.AudioPlayingAnimation import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming +import com.vitorpamplona.amethyst.ui.components.getDialogWindow import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel internal const val SKIP_SECONDS = 10 @@ -87,6 +95,7 @@ fun RenderVideoPlayer( onDialog: (() -> Unit)? = null, controllerVisible: MutableState = remember { mutableStateOf(false) }, hasBlurhash: Boolean = false, + isFullscreen: Boolean = false, accountViewModel: AccountViewModel, ) { // Hold the container size in a non-state holder so layout passes don't trigger an @@ -95,6 +104,20 @@ fun RenderVideoPlayer( val containerWidth = remember { intArrayOf(0) } val isLive = remember(mediaItem.src.videoUri) { isLiveStreaming(mediaItem.src.videoUri) } + val swipeState = remember { FullscreenSwipeControlsState() } + val context = LocalContext.current + val audioManager = remember { context.getSystemService(Context.AUDIO_SERVICE) as? AudioManager } + // Brightness is applied to the fullscreen dialog window so it auto-reverts on dismiss. + // Returns null for the inline feed player (not inside a dialog) — fine, gated by isFullscreen below. + val dialogWindow = getDialogWindow() + + // Belt-and-suspenders: clear any brightness override when this player leaves composition, so + // exiting fullscreen never leaves the screen dimmed. releaseBrightness no-ops when dialogWindow + // is null (the inline feed path) or when no override was applied, so this is safe unconditionally. + DisposableEffect(Unit) { + onDispose { swipeState.releaseBrightness(dialogWindow) } + } + WatchPlaybackErrors(controllerState) Box( @@ -115,7 +138,18 @@ fun RenderVideoPlayer( } }, ) - }, + }.then( + if (isFullscreen) { + Modifier.fullscreenSwipeControls( + state = swipeState, + audioManager = audioManager, + window = dialogWindow, + resolver = context.contentResolver, + ) + } else { + Modifier + }, + ), ) { ContentFrame( player = controllerState.controller, @@ -172,5 +206,9 @@ fun RenderVideoPlayer( isLiveStream = isLive, ) } + + if (isFullscreen) { + FullscreenSwipeLevelIndicator(swipeState) + } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt index 46cccd28ce..a9a1c43924 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/VideoViewInner.kt @@ -107,6 +107,7 @@ fun VideoViewInner( controllerVisible = controllerVisible, onDialog = onZoom, hasBlurhash = hasBlurhash, + isFullscreen = isFullscreen, accountViewModel = accountViewModel, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt new file mode 100644 index 0000000000..a3bbaf1ee1 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt @@ -0,0 +1,268 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable.controls + +import android.content.ContentResolver +import android.media.AudioManager +import android.provider.Settings +import android.view.Window +import android.view.WindowManager +import androidx.compose.animation.core.animateFloatAsState +import androidx.compose.foundation.Canvas +import androidx.compose.foundation.background +import androidx.compose.foundation.gestures.detectVerticalDragGestures +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxScope +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.alpha +import androidx.compose.ui.draw.clip +import androidx.compose.ui.graphics.StrokeCap +import androidx.compose.ui.graphics.drawscope.Stroke +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.unit.dp +import com.vitorpamplona.amethyst.commons.icons.symbols.Icon +import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols +import kotlinx.coroutines.delay + +enum class SwipeAxis { Brightness, Volume } + +private const val BRIGHTNESS_FLOOR = 0.01f +private const val AUTO_HIDE_MILLIS = 800L + +/** + * Holds the live state for the fullscreen brightness/volume swipe. A single instance is remembered + * by RenderVideoPlayer and shared between the drag [Modifier] and the [FullscreenSwipeLevelIndicator] + * overlay. All device side-effects (AudioManager / window brightness) are applied here. + */ +class FullscreenSwipeControlsState { + var axis by mutableStateOf(null) + private set + var level by mutableFloatStateOf(0f) + private set + var visible by mutableStateOf(false) + private set + + // Bumped on every drag event and on drag end; the overlay keys its auto-hide timer on this so + // the timer restarts while dragging and fires AUTO_HIDE_MILLIS after the last event. + var interactionId by mutableIntStateOf(0) + private set + + private var dragStartLevel = 0f + private var accumulatedDragPx = 0f + + fun startDrag( + axis: SwipeAxis, + audioManager: AudioManager?, + window: Window?, + resolver: ContentResolver, + ) { + this.axis = axis + accumulatedDragPx = 0f + dragStartLevel = + when (axis) { + SwipeAxis.Volume -> currentVolumeFraction(audioManager) + SwipeAxis.Brightness -> currentBrightnessFraction(window, resolver) + } + level = dragStartLevel + visible = true + interactionId++ + } + + fun onDrag( + dragAmountPx: Float, + heightPx: Float, + audioManager: AudioManager?, + window: Window?, + ) { + accumulatedDragPx += dragAmountPx + level = computeLevel(dragStartLevel, accumulatedDragPx, heightPx) + when (axis) { + SwipeAxis.Volume -> audioManager?.let { applyVolume(it, level) } + SwipeAxis.Brightness -> window?.let { applyBrightness(it, level) } + null -> Unit + } + interactionId++ + } + + fun endDrag() { + interactionId++ + } + + fun hide() { + visible = false + } + + /** + * Clears any brightness override this controller applied, restoring the window to the system + * brightness. Call from the fullscreen player's onDispose so leaving fullscreen never leaves + * the screen dimmed. + */ + fun releaseBrightness(window: Window?) { + window?.let { releaseBrightnessOverride(it) } + } +} + +private fun currentVolumeFraction(audioManager: AudioManager?): Float { + audioManager ?: return 0f + val max = audioManager.getStreamMaxVolume(AudioManager.STREAM_MUSIC) + if (max <= 0) return 0f + return audioManager.getStreamVolume(AudioManager.STREAM_MUSIC).toFloat() / max +} + +private fun currentBrightnessFraction( + window: Window?, + resolver: ContentResolver, +): Float { + val override = window?.attributes?.screenBrightness ?: -1f + if (override in 0f..1f) return override + val system = + try { + Settings.System.getInt(resolver, Settings.System.SCREEN_BRIGHTNESS) + } catch (e: Settings.SettingNotFoundException) { + 128 + } + return (system / 255f).coerceIn(0f, 1f) +} + +private fun applyVolume( + audioManager: AudioManager, + level: Float, +) { + val max = audioManager.getStreamMaxVolume(AudioManager.STREAM_MUSIC) + if (max <= 0) return + // Flag 0 = no system volume UI; we draw our own ring. + audioManager.setStreamVolume(AudioManager.STREAM_MUSIC, levelToVolumeIndex(level, max), 0) +} + +private fun applyBrightness( + window: Window, + level: Float, +) { + val params = window.attributes + params.screenBrightness = level.coerceIn(BRIGHTNESS_FLOOR, 1f) + window.attributes = params +} + +private fun releaseBrightnessOverride(window: Window) { + val params = window.attributes + params.screenBrightness = WindowManager.LayoutParams.BRIGHTNESS_OVERRIDE_NONE + window.attributes = params +} + +/** + * Vertical-drag handler for the fullscreen video surface. Left half of the surface controls + * brightness, right half controls volume. Must be a separate [pointerInput] from the existing + * tap/double-tap handler so taps still work. + */ +fun Modifier.fullscreenSwipeControls( + state: FullscreenSwipeControlsState, + audioManager: AudioManager?, + window: Window?, + resolver: ContentResolver, +): Modifier = + pointerInput(state, audioManager, window, resolver) { + detectVerticalDragGestures( + onDragStart = { offset -> + val axis = if (offset.x < size.width / 2f) SwipeAxis.Brightness else SwipeAxis.Volume + state.startDrag(axis, audioManager, window, resolver) + }, + onVerticalDrag = { _, dragAmount -> + state.onDrag(dragAmount, size.height.toFloat(), audioManager, window) + }, + onDragEnd = { state.endDrag() }, + onDragCancel = { state.endDrag() }, + ) + } + +/** Centered ring + glyph that appears while swiping and fades out shortly after the drag ends. */ +@Composable +fun BoxScope.FullscreenSwipeLevelIndicator(state: FullscreenSwipeControlsState) { + LaunchedEffect(state.interactionId) { + if (state.visible) { + delay(AUTO_HIDE_MILLIS) + state.hide() + } + } + + val alpha by animateFloatAsState(if (state.visible) 1f else 0f, label = "swipeIndicatorAlpha") + if (alpha <= 0f) return + + val axis = state.axis ?: return + val level = state.level + val ringColor = MaterialTheme.colorScheme.onBackground + val trackColor = ringColor.copy(alpha = 0.25f) + val backdrop = MaterialTheme.colorScheme.background.copy(alpha = 0.5f) + + Box( + modifier = + Modifier + .align(Alignment.Center) + .size(110.dp) + .alpha(alpha) + .clip(CircleShape) + .background(backdrop), + contentAlignment = Alignment.Center, + ) { + Canvas(modifier = Modifier.fillMaxSize().padding(16.dp)) { + val stroke = Stroke(width = 6.dp.toPx(), cap = StrokeCap.Round) + drawArc( + color = trackColor, + startAngle = -90f, + sweepAngle = 360f, + useCenter = false, + style = stroke, + ) + drawArc( + color = ringColor, + startAngle = -90f, + sweepAngle = 360f * level.coerceIn(0f, 1f), + useCenter = false, + style = stroke, + ) + } + + val symbol = + when (axis) { + SwipeAxis.Brightness -> MaterialSymbols.BrightnessMedium + SwipeAxis.Volume -> + if (level <= 0f) MaterialSymbols.AutoMirrored.VolumeOff else MaterialSymbols.AutoMirrored.VolumeUp + } + Icon( + symbol = symbol, + contentDescription = null, + tint = ringColor, + modifier = Modifier.size(36.dp), + ) + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt new file mode 100644 index 0000000000..acb4fc1677 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable.controls + +import kotlin.math.roundToInt + +/** + * Maps a vertical drag to a 0..1 level. Dragging up (negative accumulated pixels) increases the + * level; dragging down decreases it. A drag spanning the full element height covers the entire + * 0..1 range. The result is clamped to 0..1. + */ +fun computeLevel( + startLevel: Float, + accumulatedDragPx: Float, + heightPx: Float, +): Float { + if (heightPx <= 0f) return startLevel.coerceIn(0f, 1f) + return (startLevel - accumulatedDragPx / heightPx).coerceIn(0f, 1f) +} + +/** Clamps [level] to 0..1, then rounds it to a discrete stream-volume index in 0..max. Returns 0 when max <= 0. */ +fun levelToVolumeIndex( + level: Float, + max: Int, +): Int { + if (max <= 0) return 0 + return (level.coerceIn(0f, 1f) * max).roundToInt() +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt new file mode 100644 index 0000000000..1bca4e82fd --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable.controls + +import org.junit.Assert.assertEquals +import org.junit.Test + +class FullscreenSwipeMathTest { + @Test + fun dragUpIncreasesLevel() { + // Drag up 500px (negative) on a 1000px screen from 0.2 -> +0.5 = 0.7 + assertEquals(0.7f, computeLevel(0.2f, -500f, 1000f), 0.0001f) + } + + @Test + fun dragDownDecreasesLevel() { + assertEquals(0.3f, computeLevel(0.8f, 500f, 1000f), 0.0001f) + } + + @Test + fun clampsToOne() { + assertEquals(1f, computeLevel(0.9f, -500f, 1000f), 0.0001f) + } + + @Test + fun clampsToZero() { + assertEquals(0f, computeLevel(0.1f, 500f, 1000f), 0.0001f) + } + + @Test + fun zeroHeightReturnsStartClamped() { + assertEquals(0.5f, computeLevel(0.5f, -100f, 0f), 0.0001f) + } + + @Test + fun zeroHeightClampsOutOfRangeStartLevel() { + assertEquals(1f, computeLevel(1.5f, -100f, 0f), 0.0001f) + assertEquals(0f, computeLevel(-0.3f, 100f, 0f), 0.0001f) + } + + @Test + fun volumeIndexExactMidpoint() { + assertEquals(5, levelToVolumeIndex(0.5f, 10)) + } + + @Test + fun volumeIndexFull() { + assertEquals(15, levelToVolumeIndex(1f, 15)) + } + + @Test + fun volumeIndexZeroLevel() { + assertEquals(0, levelToVolumeIndex(0f, 15)) + } + + @Test + fun volumeIndexZeroMaxGuard() { + assertEquals(0, levelToVolumeIndex(0.5f, 0)) + } +} diff --git a/commons/src/commonMain/composeResources/font/material_symbols_outlined.ttf b/commons/src/commonMain/composeResources/font/material_symbols_outlined.ttf index f8902f46e3d0d565bbb138b9204bcdf565d42a4e..733b19cde6283483dd86d761b7043208ddf0dd99 100644 GIT binary patch delta 3972 zcmbW1dvsGp9>>3P@7$zGLz>zo4QZgUrM8L+NK08krB5mk=>vK=qC#yD)E2w)SXfun z&~9NBS@&F4JuaehfCH;y*Xp5y4?KddEV2ryh1J?sg<&SN7WDW57ct8lu zsTg-tUfIHFKx;kCT2wJNZ|=J870&=|S=2vJwW{i2n0{!czMuM~)wNZ1PftHIhx8qQ zuKYf-KXJZxH#t-F{v}n*y>;}Dg&e*}ZQPQYHTS)ALfQ{VR^Zk7OK8IJ$62{}bAMxI9Zh>18}~N#*A**QR(Lje20z!JrD^Z#l{;2O zP7Rto8@ee})w6fNbt<{O!m)n1OD&Mgy~eERQ6q@;YK<{wQ3T|&6fcT(q8mv_$3!5v z#o%kao_Sg}zM3a^ubWmq?fGf%ng#eLDMw*$ZaxpB=bk)g2$sshqRBG{5xavXmKR6=lzq z9W1}CTrU5r!ccKf#S3#&<}RE2{ycHs>Ul>i$5cL8`3l{+n(o56zuz|c;>v{yQ!m5` z;(i*z=}G5DYi+KxyJb`J6hWl(Z8`G#EiC?4np8v6^r(^TZ%kpkTT0)kWz@X==0tY2 z;tsl}xkK(A_s-M^rBs)?N@{27DYrFsR_c51M`X($yDrr;`oYu<@^yPs=IBOkbp7O7 z=6ce#W0b@7m}@fNI#0#s%5jZMsY|h@Q0k{7q>NG*az>mXXJ<-} zSw1p6@`aIiJO1god1So2Y0tGujgD2xio=)uL2?R^+?!nB$d*6dlRhqF|A#$nUt<5* zo@HBOE46l7J8gfkO|U+Zm|*!Nu{!a;mTJ|DB?KgVXYtEvtr>>w1ebX$5Z@t}wkGT8 zS+Tp=BKfzi8FGEA)be!eST!-oh4a2IN`qTuu42?41Jw7petM%T4m zS-N7`;^oU1)-Lwkzp~bT#YoG?pL`(N7?`5a+oKj{WP#o+n=fzuYzkW^w}19oij$3| z5Yt%(%VgIw3Y(2%6WC;SGn*bc1bNi4F$Q7+{Tvj{jAI7`dHlOvHh=Li z`QVq8hMfcqx_@0=%b2!1xHoG(_}G)+?;E;;rB_CxOrD11~=WKF4C-)NXy=awEQHbRrQd3G}!bbq>ZZ~{lx|8*)5PZ)7bVVNW0o0z4kby z77l4&Go<}wJNO2qk557ReX0?OPB*SPIelBpyh)164Ijjr8*Y`2cuIMPToH=bINY*s2}Di$hNk$waf(?|z| zfghuvK!2A$cz0lPp<+=i(O1-3*bGl0y3x(%sx>?f+w*_XB6zSlPo2?_Y`?U+C=4x3 z9_r()PAmMU{h?l;F&gL%L?IYWM#0`c3acs9Yw{WeT(a4$xD=Nbpc*UZygsR?<9J68 z%mWlNFgm$D|rLIK-UTxvH zNM9P-;Yv|w(N_ipm*2^p)opqs5fSpWaDt_;h);t5U>5~J&f$E#Gl$wpA>3uKiS?Mf|Qn)nU*FY z)Y}{KnR|MBPxtoH?oB4A$z+x@F4wfTn@kcZRU2ZoGIp2+BOSMPzI6D3Xv6k|*e+d$ zv4QU291RBhfUWY;#}gK$r+bvXpY&eBc*DCs`U=BM~qy(uYfE)X1n}k?2T#TZ`ygwHDUeF1>DIpL*y4MwzV8b+9G~ zO*ZW{47`8!b+aMwZ4a zKr)Ds0ck&28i)znHBqgswJF3JKY>ZnnazYAy~0G1DGgAX=(8o6&DA<}c~t_Egu@XM l&_hTcj*u80XnatS(x?$679hjX(GQe$k1}b`qf8gS^?yuY8fO3i delta 1373 zcmX|B3rv$&6#j0x7evq&?F0V)Kz$$*Wk`)kMCDnYl@U8sk4~!071XIZt9aAGt-7YTKpPYOC^PTgZ$Gtaq=N+N? zoL~n5;KLURMUo{uPHNfvIdHWeAXUjZv6&yu-Jk(_ML;%evV`elGnT~wPVUEYOIB=_ z<4O5m;HHAxJ=V3>58(Z%6}WYp+vfZ-tL<9oH&HzPDd3)3$o<>?Dm%jCuXF$JMbH@DJrcfEU8_$j~C|!TS|*KVK7uZ3+PjTU0hqd!dfsl$|wg~M|ly&9PoJH zq2dKb?Wx6OFRu|V|XbA4!^a_tm|#0k`!>en#b!_Sp&*e+^vwBcP0SD%l;o^G@ZSnJkawW3sfcTkWiE3zpe~W0GR_#$1Sf zH?}EuC{7cX9`}CSfw*h&Q{zkGdlEbnW+zl8oKJXekQ;&wdPBOwYS?I~Gjtd(86Fv) zCweEACZ0@sCuwKW&1AP^L-LxG2`PCgS5hNWKTI7=i%hFaYd20d<{I0LgNsrZZB9qJ zIsLvV()6==j`^VZ*NjPt8K#V%GW|0PGo?(ICB$;j@**oOYkRgVdr9_@>}NR%@0j*r?Kbl2&E&MIGv>8H@AP8FFE58w8j$rnC z>9^l+vF~}`d48T!RcFY=kA2HE|M=Q9KWMao=BXy$S1+CEoDw#o{#N}`U8Fvzp027; znUyZ3OSM52uH5D$SN!6W?{iPVPm6$^tg^(G@BNK>{1Q1a=b1G`C39U%R0W{$r?ve~*!E6kX6tWoM zm`~n=jYOjvBKk-WZj)G?z!zxXcT^-2A>mT1>>yt|Wzo`Zw|Z$#w~pjFbGmzE&VVxw zKt>)A;1Mu&ATcA9UeQ+u#GXvlcbur^SA1L%TF zpmBMiNmD^no`V|afu{3EA#)#Sb{6OoCFt^8(86lam9IcoodsRXu?-QRniA!wTo^f*s` zvIX?QFz6K>=&dE7zl{YQf>O61z?jo9@+<{ ze*?DgQ?P_$Fw-@#9B(jdE7%Gb*h=>pu(B4g3Jz`(z$y=bRVRbhT>z`+SVJ4wmt(=2 ykAZzX3#{!fSUdN1-Us_$0rungtpojzfque8#>K6Dpr2lz;&6Dec88a{XVbsMMVRdX diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/icons/symbols/MaterialSymbols.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/icons/symbols/MaterialSymbols.kt index cf17505499..88c122bfdd 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/icons/symbols/MaterialSymbols.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/icons/symbols/MaterialSymbols.kt @@ -46,6 +46,7 @@ object MaterialSymbols { val BookmarkAdd = MaterialSymbol("\uE598") val BookmarkBorder = MaterialSymbol("\uE8E7") val BookmarkRemove = MaterialSymbol("\uE59A") + val BrightnessMedium = MaterialSymbol("\uE1AE") val Forward10 = MaterialSymbol("\uE056") val Replay10 = MaterialSymbol("\uE059") val CalendarMonth = MaterialSymbol("\uEBCC") From f2df23cfa664022e958eee73d613debe99b9ec27 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 4 Jun 2026 20:52:35 +0200 Subject: [PATCH 7/8] Code review: - fix(player): harden fullscreen swipe controls and brightness lifecycle --- .../controls/FullscreenSwipeControls.kt | 88 +++++++++++++------ .../ui/components/ZoomableContentDialog.kt | 6 ++ 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt index a3bbaf1ee1..4e63f5e9c7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt @@ -43,6 +43,7 @@ import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -54,6 +55,8 @@ import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.collectLatest +import kotlin.math.roundToInt enum class SwipeAxis { Brightness, Volume } @@ -81,6 +84,16 @@ class FullscreenSwipeControlsState { private var dragStartLevel = 0f private var accumulatedDragPx = 0f + // Max volume is device-invariant; cache it at drag start so the per-frame drag path doesn't + // re-query AudioManager on every pointer event. + private var maxVolume = 0 + + // Last discrete value actually pushed to the device this drag. Continuous finger movement maps + // to the same volume index / brightness step across several frames; skipping unchanged writes + // avoids redundant AudioManager calls and window-attribute relayouts with no visible effect. + private var lastVolumeIndex = -1 + private var lastBrightnessStep = -1 + fun startDrag( axis: SwipeAxis, audioManager: AudioManager?, @@ -89,11 +102,17 @@ class FullscreenSwipeControlsState { ) { this.axis = axis accumulatedDragPx = 0f - dragStartLevel = - when (axis) { - SwipeAxis.Volume -> currentVolumeFraction(audioManager) - SwipeAxis.Brightness -> currentBrightnessFraction(window, resolver) + when (axis) { + SwipeAxis.Volume -> { + maxVolume = audioManager?.getStreamMaxVolume(AudioManager.STREAM_MUSIC) ?: 0 + lastVolumeIndex = -1 + dragStartLevel = currentVolumeFraction(audioManager, maxVolume) } + SwipeAxis.Brightness -> { + lastBrightnessStep = -1 + dragStartLevel = currentBrightnessFraction(window, resolver) + } + } level = dragStartLevel visible = true interactionId++ @@ -108,13 +127,32 @@ class FullscreenSwipeControlsState { accumulatedDragPx += dragAmountPx level = computeLevel(dragStartLevel, accumulatedDragPx, heightPx) when (axis) { - SwipeAxis.Volume -> audioManager?.let { applyVolume(it, level) } - SwipeAxis.Brightness -> window?.let { applyBrightness(it, level) } + SwipeAxis.Volume -> audioManager?.let { applyVolumeIfChanged(it) } + SwipeAxis.Brightness -> window?.let { applyBrightnessIfChanged(it) } null -> Unit } interactionId++ } + private fun applyVolumeIfChanged(audioManager: AudioManager) { + if (maxVolume <= 0) return + val index = levelToVolumeIndex(level, maxVolume) + if (index == lastVolumeIndex) return + lastVolumeIndex = index + // Flag 0 = no system volume UI; we draw our own ring. + audioManager.setStreamVolume(AudioManager.STREAM_MUSIC, index, 0) + } + + private fun applyBrightnessIfChanged(window: Window) { + val target = level.coerceIn(BRIGHTNESS_FLOOR, 1f) + // Quantize to the panel's 0..255 range; sub-step movement is imperceptible and would + // otherwise reassign window.attributes (a relayout) every frame for no visible change. + val step = (target * 255f).roundToInt() + if (step == lastBrightnessStep) return + lastBrightnessStep = step + applyBrightness(window, target) + } + fun endDrag() { interactionId++ } @@ -133,9 +171,11 @@ class FullscreenSwipeControlsState { } } -private fun currentVolumeFraction(audioManager: AudioManager?): Float { +private fun currentVolumeFraction( + audioManager: AudioManager?, + max: Int, +): Float { audioManager ?: return 0f - val max = audioManager.getStreamMaxVolume(AudioManager.STREAM_MUSIC) if (max <= 0) return 0f return audioManager.getStreamVolume(AudioManager.STREAM_MUSIC).toFloat() / max } @@ -155,22 +195,12 @@ private fun currentBrightnessFraction( return (system / 255f).coerceIn(0f, 1f) } -private fun applyVolume( - audioManager: AudioManager, - level: Float, -) { - val max = audioManager.getStreamMaxVolume(AudioManager.STREAM_MUSIC) - if (max <= 0) return - // Flag 0 = no system volume UI; we draw our own ring. - audioManager.setStreamVolume(AudioManager.STREAM_MUSIC, levelToVolumeIndex(level, max), 0) -} - private fun applyBrightness( window: Window, - level: Float, + brightness: Float, ) { val params = window.attributes - params.screenBrightness = level.coerceIn(BRIGHTNESS_FLOOR, 1f) + params.screenBrightness = brightness window.attributes = params } @@ -208,11 +238,19 @@ fun Modifier.fullscreenSwipeControls( /** Centered ring + glyph that appears while swiping and fades out shortly after the drag ends. */ @Composable fun BoxScope.FullscreenSwipeLevelIndicator(state: FullscreenSwipeControlsState) { - LaunchedEffect(state.interactionId) { - if (state.visible) { - delay(AUTO_HIDE_MILLIS) - state.hide() - } + // Launch once on the stable state and watch interactionId via a snapshotFlow instead of keying + // the effect on it: collectLatest restarts the auto-hide delay on each drag event, and reading + // interactionId here (not as a composition key) avoids re-keying the effect every frame. The + // indicator still recomposes per frame to redraw the arc as level changes — fine for a transient + // drag overlay. + LaunchedEffect(state) { + snapshotFlow { state.interactionId } + .collectLatest { + if (state.visible) { + delay(AUTO_HIDE_MILLIS) + state.hide() + } + } } val alpha by animateFloatAsState(if (state.visible) 1f else 0f, label = "swipeIndicatorAlpha") diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt index e53321661e..93ab093d99 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt @@ -191,12 +191,18 @@ fun ZoomableImageDialog( val dialogWindow = getDialogWindow() if (activityWindow != null && dialogWindow != null) { + // Preserve any brightness override already applied to the dialog window (e.g. by the + // fullscreen swipe controls). This block re-runs on recomposition (orientation change + // re-reads `orientation` above), and copying the activity attributes would otherwise + // reset screenBrightness and snap the user's brightness back mid-session. + val currentBrightness = dialogWindow.attributes.screenBrightness val attributes = WindowManager.LayoutParams() attributes.copyFrom(activityWindow.attributes) attributes.type = dialogWindow.attributes.type // Disable the system dim so the thumbnail stays visible behind the growing dialog. attributes.dimAmount = 0f attributes.flags = attributes.flags and WindowManager.LayoutParams.FLAG_DIM_BEHIND.inv() + attributes.screenBrightness = currentBrightness dialogWindow.attributes = attributes } From 54caab3520e7c5646a2225abca6649bc58fa311e Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 4 Jun 2026 22:42:58 +0200 Subject: [PATCH 8/8] Sync video mute with fullscreen volume swipe --- .../playback/composable/RenderVideoPlayer.kt | 13 ++++++- .../controls/FullscreenSwipeControls.kt | 34 ++++++++++++++++--- .../controls/FullscreenSwipeMath.kt | 25 ++++++++++++++ .../controls/FullscreenSwipeMathTest.kt | 31 +++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt index 5c488fde80..2cae673cab 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt @@ -111,6 +111,15 @@ fun RenderVideoPlayer( // Returns null for the inline feed player (not inside a dialog) — fine, gated by isFullscreen below. val dialogWindow = getDialogWindow() + // Sync the per-video mute (Media3 player volume) with the volume swipe. Mirrors the mute + // button's handler exactly, so its listener-driven icon flips when the swipe mutes/unmutes, + // and the global default carries to the next video. + val isVideoMuted = { controllerState.controller.volume < 0.001f } + val setVideoMuted = { mute: Boolean -> + DEFAULT_MUTED_SETTING.value = mute + controllerState.controller.volume = if (mute) 0f else 1f + } + // Belt-and-suspenders: clear any brightness override when this player leaves composition, so // exiting fullscreen never leaves the screen dimmed. releaseBrightness no-ops when dialogWindow // is null (the inline feed path) or when no override was applied, so this is safe unconditionally. @@ -145,6 +154,8 @@ fun RenderVideoPlayer( audioManager = audioManager, window = dialogWindow, resolver = context.contentResolver, + isMuted = isVideoMuted, + setMuted = setVideoMuted, ) } else { Modifier @@ -208,7 +219,7 @@ fun RenderVideoPlayer( } if (isFullscreen) { - FullscreenSwipeLevelIndicator(swipeState) + FullscreenSwipeLevelIndicator(swipeState, isMuted = isVideoMuted) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt index 4e63f5e9c7..32585ceb51 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeControls.kt @@ -123,17 +123,36 @@ class FullscreenSwipeControlsState { heightPx: Float, audioManager: AudioManager?, window: Window?, + isMuted: () -> Boolean, + setMuted: (Boolean) -> Unit, ) { accumulatedDragPx += dragAmountPx level = computeLevel(dragStartLevel, accumulatedDragPx, heightPx) when (axis) { - SwipeAxis.Volume -> audioManager?.let { applyVolumeIfChanged(it) } + SwipeAxis.Volume -> { + audioManager?.let { applyVolumeIfChanged(it) } + applyMuteSync(isMuted, setMuted) + } SwipeAxis.Brightness -> window?.let { applyBrightnessIfChanged(it) } null -> Unit } interactionId++ } + // Directional mute sync: dragging up unmutes a muted video; reaching zero mutes it. Kept outside + // the AudioManager branch so per-video mute still tracks the gesture even if device volume is + // unavailable. + private fun applyMuteSync( + isMuted: () -> Boolean, + setMuted: (Boolean) -> Unit, + ) { + when (muteActionFor(level, movedUp = accumulatedDragPx < 0f, isMuted = isMuted())) { + MuteAction.Mute -> setMuted(true) + MuteAction.Unmute -> setMuted(false) + MuteAction.None -> Unit + } + } + private fun applyVolumeIfChanged(audioManager: AudioManager) { if (maxVolume <= 0) return val index = levelToVolumeIndex(level, maxVolume) @@ -220,7 +239,11 @@ fun Modifier.fullscreenSwipeControls( audioManager: AudioManager?, window: Window?, resolver: ContentResolver, + isMuted: () -> Boolean, + setMuted: (Boolean) -> Unit, ): Modifier = + // isMuted/setMuted are intentionally not pointerInput keys: they change identity every + // recomposition but read live state, so adding them would restart the gesture for no reason. pointerInput(state, audioManager, window, resolver) { detectVerticalDragGestures( onDragStart = { offset -> @@ -228,7 +251,7 @@ fun Modifier.fullscreenSwipeControls( state.startDrag(axis, audioManager, window, resolver) }, onVerticalDrag = { _, dragAmount -> - state.onDrag(dragAmount, size.height.toFloat(), audioManager, window) + state.onDrag(dragAmount, size.height.toFloat(), audioManager, window, isMuted, setMuted) }, onDragEnd = { state.endDrag() }, onDragCancel = { state.endDrag() }, @@ -237,7 +260,10 @@ fun Modifier.fullscreenSwipeControls( /** Centered ring + glyph that appears while swiping and fades out shortly after the drag ends. */ @Composable -fun BoxScope.FullscreenSwipeLevelIndicator(state: FullscreenSwipeControlsState) { +fun BoxScope.FullscreenSwipeLevelIndicator( + state: FullscreenSwipeControlsState, + isMuted: () -> Boolean, +) { // Launch once on the stable state and watch interactionId via a snapshotFlow instead of keying // the effect on it: collectLatest restarts the auto-hide delay on each drag event, and reading // interactionId here (not as a composition key) avoids re-keying the effect every frame. The @@ -294,7 +320,7 @@ fun BoxScope.FullscreenSwipeLevelIndicator(state: FullscreenSwipeControlsState) when (axis) { SwipeAxis.Brightness -> MaterialSymbols.BrightnessMedium SwipeAxis.Volume -> - if (level <= 0f) MaterialSymbols.AutoMirrored.VolumeOff else MaterialSymbols.AutoMirrored.VolumeUp + if (isMuted() || level <= 0f) MaterialSymbols.AutoMirrored.VolumeOff else MaterialSymbols.AutoMirrored.VolumeUp } Icon( symbol = symbol, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt index acb4fc1677..2e7999b072 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMath.kt @@ -44,3 +44,28 @@ fun levelToVolumeIndex( if (max <= 0) return 0 return (level.coerceIn(0f, 1f) * max).roundToInt() } + +/** The mute change a volume swipe should trigger on the per-video player. */ +enum class MuteAction { Mute, Unmute, None } + +/** + * Directional mute sync for the volume swipe. + * + * - Reaching zero mutes the video (no-op if already muted). + * - Dragging the finger up ([movedUp], i.e. net upward from where the drag started) unmutes a muted + * video — so a muted video pinned at max device volume still unmutes even though [level] can't rise. + * - A downward swipe that stays above zero leaves the mute state untouched. + * + * [movedUp] is the drag direction, not a level comparison, so the clamp at level 1.0 doesn't swallow + * the unmute intent. + */ +fun muteActionFor( + level: Float, + movedUp: Boolean, + isMuted: Boolean, +): MuteAction = + when { + level <= 0f -> if (isMuted) MuteAction.None else MuteAction.Mute + isMuted && movedUp -> MuteAction.Unmute + else -> MuteAction.None + } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt index 1bca4e82fd..9eb703c166 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/controls/FullscreenSwipeMathTest.kt @@ -75,4 +75,35 @@ class FullscreenSwipeMathTest { fun volumeIndexZeroMaxGuard() { assertEquals(0, levelToVolumeIndex(0.5f, 0)) } + + @Test + fun reachingZeroWhileUnmutedMutes() { + assertEquals(MuteAction.Mute, muteActionFor(level = 0f, movedUp = false, isMuted = false)) + } + + @Test + fun reachingZeroWhileMutedIsNoop() { + assertEquals(MuteAction.None, muteActionFor(level = 0f, movedUp = false, isMuted = true)) + } + + @Test + fun movingUpWhileMutedUnmutes() { + assertEquals(MuteAction.Unmute, muteActionFor(level = 0.5f, movedUp = true, isMuted = true)) + } + + @Test + fun movingUpAtMaxWhileMutedStillUnmutes() { + // Device volume pinned at 1.0 can't rise, but the upward drag still expresses intent. + assertEquals(MuteAction.Unmute, muteActionFor(level = 1f, movedUp = true, isMuted = true)) + } + + @Test + fun movingUpWhileUnmutedIsNoop() { + assertEquals(MuteAction.None, muteActionFor(level = 0.5f, movedUp = true, isMuted = false)) + } + + @Test + fun movingDownAboveZeroWhileMutedIsNoop() { + assertEquals(MuteAction.None, muteActionFor(level = 0.5f, movedUp = false, isMuted = true)) + } }