ShreyashKore / wonderous_compose

Wonderous Compose is a port of Wonderous in Compose Multiplatform.
https://shreyashkore.github.io/wonderous-compose-wasm/
MIT License
87 stars 1 forks source link

Bug: Map bounds are incorrectly calculated, causing visible tiles to not load #18

Closed oblakr24 closed 8 hours ago

oblakr24 commented 1 week ago

First of all, thank you for this amazing project!

The issue is simple: the map view wrapper around the Leaflet map seems to be calculating the bounds of the canvas incorrectly.

If you try to scroll down (i.e. move south), then you quickly enter a gray region where the tiles have not yet loaded. These tiles will never load until you scroll further down (indicating that they initially do not get requested).

Screenshot 2024-07-05 at 16 15 32
ShreyashKore commented 1 week ago

This issue seems to be connected to how Leaflet works. The same issue is present in React Leaflet too. I tried applying the fix mentioned in the accepted answer, but the issue still persists.

oblakr24 commented 1 week ago

Thanks @ShreyashKore . What I did instead, is port over the desktop map implementation to wasm, by changing the caching decorator and the downloader decorator (the only two classes with JVM dependencies or coroutine dependencies not working on wasm). And it works surprisingly well. Thus I do not see a need to use the Leaflet on web anymore; do you have any input here or do you see any reservations? It only increases my static size by 0.3 MB.

The parts that needed changing were (note: the caching part was quickly done for proof-of-concept, but is likely not ideal, as it just stores encoded strings in local storage):

decorateWithDistinctDownloader:

private sealed interface Message<K, T> {
    class DownloadComplete<K, T>(val key: K, val result: T) : Message<K, T>
    class DownloadFail<K, T>(val key: K, val exception: Throwable) : Message<K, T>
}

fun <K, T> ContentRepository<K, T>.decorateWithDistinctDownloader(
    scope: CoroutineScope
): ContentRepository<K, T> {
    val origin = this
    val mutex = Mutex()
    val mapKeyToRequests: MutableMap<K, MutableList<CompletableDeferred<T>>> = mutableMapOf()

    return object : ContentRepository<K, T> {
        override suspend fun loadContent(key: K): T {
            val deferred = CompletableDeferred<T>()
            val isRequestNew: Boolean

            mutex.withLock {
                isRequestNew = mapKeyToRequests.getOrPut(key) { mutableListOf() }.isEmpty()
                mapKeyToRequests[key]!!.add(deferred)
            }

            if (isRequestNew) {
                scope.launch {
                    val result = try {
                        val res = origin.loadContent(key)
                        Message.DownloadComplete(key, res)
                    } catch (e: Throwable) {
                        Message.DownloadFail(key, e)
                    }

                    mutex.withLock {
                        mapKeyToRequests[key]?.let { requests ->
                            when (result) {
                                is Message.DownloadComplete -> {
                                    requests.forEach { it.complete(result.result) }
                                }
                                is Message.DownloadFail -> {
                                    requests.forEach { it.completeExceptionally(result.exception) }
                                }
                            }
                            mapKeyToRequests.remove(key)
                        }
                    }
                }
            }
            return deferred.await()
        }
    }
}

decorateWithDiskCache:


interface CacheDir {
    fun ensureExists()

    fun dirExists(): Boolean

    fun resolve(path: String): CacheFile
}

interface CacheFile {
    fun readBytes(): ByteArray?
    fun writeBytes(bytes: ByteArray)
}

object WebCacheImpl {
    // TODO: Finalize implementation: move to something other than localStorage?
    fun create(dirName: String): CacheDir {
        window.localStorage.clear()
        return object : CacheDir {
            override fun ensureExists() {
                // Nothing to do here?
            }

            override fun dirExists(): Boolean {
                return true // Nothing to check here?
            }

            override fun resolve(path: String): CacheFile {

                val string = window.localStorage.getItem(path)?.takeIf { it.isNotBlank() }
                return WebCacheFileImpl(contentString = string, key = path)
            }

        }
    }
}

@OptIn(ExperimentalEncodingApi::class)
class WebCacheFileImpl(val contentString: String?, val key: String): CacheFile {
    override fun readBytes(): ByteArray? {
        println("Attempting to read string with len ${contentString?.length}: $contentString")
        return contentString?.let { it.decodeBase64Bytes().also { println("read is ${it.encodeBase64()}") } }
    }

    override fun writeBytes(bytes: ByteArray) {
        println("Writing under key $key: ${bytes.encodeBase64()}")
        window.localStorage.setItem(key, bytes.encodeBase64())
    }
}

fun ContentRepository<Tile, ByteArray>.decorateWithDiskCache(
    backgroundScope: CoroutineScope,
    cacheDir: CacheDir,
): ContentRepository<Tile, ByteArray> {

    class FileSystemLock

    val origin = this
    val locksCount = 100
    val locks = Array(locksCount) { FileSystemLock() }

    fun getLock(key: Tile) = locks[key.hashCode() % locksCount]

    return object : ContentRepository<Tile, ByteArray> {
        init {
            cacheDir.ensureExists()
        }

        override suspend fun loadContent(key: Tile): ByteArray {
            if (!cacheDir.dirExists()) {
                return origin.loadContent(key)
            }
            val file = with(key) {
                cacheDir.resolve("tile-$zoom-$x-$y.png")
            }

            val fromCache: ByteArray? = synchronized(getLock(key)) {
                file.readBytes()
            }

            val result = if (fromCache != null) {
                fromCache
            } else {
                println("Cache miss for $key")
                val image = origin.loadContent(key)
                backgroundScope.launch {
                    synchronized(getLock(key)) {
                        // save to cacheDir
                        file.writeBytes(image)
                    }
                }
                image
            }
            return result
        }
    }
}
ShreyashKore commented 1 week ago

One thing to note is that this issue resolves automatically once you resize the window. Could it be that Leaflet is still using stale dimension values from the first layout?

https://github.com/ShreyashKore/wonderous_compose/assets/46917404/28d8fc1f-145b-4f2f-a31e-ece6c5337550

ShreyashKore commented 1 week ago

That is a good idea. I will need to look into the desktop MapView code in depth to provide any feedback (I had simply copy pasted that from the Compose repository :P).

Also, In my opinion, this screen should use platform-specific components, to showcase Compose's interoperability features. Any idea how we can fix this issue while keeping Leaflet?

ShreyashKore commented 1 week ago

First of all, thank you for this amazing project!

The issue is simple: the map view wrapper around the Leaflet map seems to be calculating the bounds of the canvas incorrectly.

If you try to scroll down (i.e. move south), then you quickly enter a gray region where the tiles have not yet loaded. These tiles will never load until you scroll further down (indicating that they initially do not get requested).

Screenshot 2024-07-05 at 16 15 32

And thanks for your kind words! I am happy that u r finding Wonderous helpful. I am planning to add more features in it. Feel free to share ur thoughts!

Shusshu commented 6 days ago

Ported in https://github.com/ShreyashKore/wonderous_compose/pull/19 - Thx @oblakr24