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

IntDiffOptRleEncoder's toUint8Array returns a different result each time. #60

Closed
ObuchiYuki opened this issue Mar 20, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@ObuchiYuki
Copy link

ObuchiYuki commented Mar 20, 2023

Describe the bug
After writing the content to IntDiffOptRleEncoder (also UintOptRleEncoder, IncUintOptRleEncoder), when reading the content with toUint8Array(), the content is different each time due to flush.

Expected behavior
Since toUint8Array() is obviously a function for reading values, its contents should not change with each call.

To fix
IntDiffOptRleEncoder should have a flag like mutated. In that case, the implementation would be as follows

export class IntDiffOptRleEncoder {
  constructor () {
    this.encoder = new Encoder()
    /**
     * @type {number}
     */
    this.s = 0
    this.count = 0
    this.diff = 0
    this.mutated = false
  }

  /**
   * @param {number} v
   */
  write (v) {
    this.mutated = true
    if (this.diff === v - this.s) {
      this.s = v
      this.count++
    } else {
      flushIntDiffOptRleEncoder(this)
      this.count = 1
      this.diff = v - this.s
      this.s = v
    }
  }

  toUint8Array () {
    if (this.mutated) {
      flushIntDiffOptRleEncoder(this)
      this.mutated = false
    }
    return toUint8Array(this.encoder)
  }
}

Appendices
In yjs, there is no part of IntDiffOptRleEncoder that calls toUint8Array() twice.
I also confirmed that IntDiffOptRleEncoder with this modification passes all tests in yjs.

(I am personally working on yjs for Swift yswift. I discovered this in debugging).

@ObuchiYuki ObuchiYuki added the bug Something isn't working label Mar 20, 2023
@dmonad
Copy link
Owner

dmonad commented Mar 20, 2023

Hi @ObuchiYuki,

I should add more documentation on this. The idea is that you call toUint8Array only once to serialize the data. Serializing it several times is not supposed to work. It's also not supported by the other RLE encoders - to decrease complexity. Instead, you could create a new RLE encoder if you want to encode new data.

What's the use-case for calling toUint8Array twice?

FYI: We are also working on a Swift binding based on uniffi: https://github.com/y-crdt/y-uniffi

@dmonad dmonad closed this as completed in cac4215 Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants