Skip to content

Introduce handle_cbor #848

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rikhuijzer
Copy link
Contributor

@rikhuijzer rikhuijzer commented Mar 24, 2025

Small follow up on #846. This PR suggest to change two things:

First, it introduces a handle_cbor function that handles the cbor de(serialization) and error handling. I verified in 032de2d that the function generalizes well (in that commit I used it to take arguments to the path-utils.bounds. 032de2d passes all the tests too, but needs a bigger rewrite to get good speed, so I'll do that in another PR).

The handle_cbor function might look complex, but essentially what it is is a parametric function over types T (the arguments struct CubicExtremaArgs), F (the processor function cubic_extrema), and U (the return type of F, so the return type of cubic_extrema). Since it is a parametric function, all these types will be inferred during the Rust compilation. The compiler will create one function for each combination of these types that is used in the code. So this parametric function should not affect CeTZ runtime since it's all compiled into the wasm binary.

Second, I got rid of vector_add and vector_scale in order to improve performance. Long-term we should probably get these functions from a library as you mentioned, but for now it was faster to avoid the intermediate vectors. This change is a few percent faster.

One small note on the arguments of cubic_point. I used pointers now for the Point. This is a tiny bit faster than cloning the Point vector. All in all it didn't seem to matter much though. The vectors are so small that cloning is roughly as fast as passing pointers. At least in this case.

@rikhuijzer
Copy link
Contributor Author

rikhuijzer commented Mar 24, 2025

While working on something else, I figured that error handling wasn't done well. In fa05609, the code is now returning a String in the error case. This way, parsing errors during the cbor parsing properly bubble up. For example, when on purpose passing in a wrong arg in c2:

#let cubic-extrema(s, e, c1, c2) = {
  let args = (s: s, e: e, c1: c1, c2: 1)
  let encoded = cbor.encode(args)
  let decoded = cbor(cetz-core.cubic_extrema_func(encoded))
  decoded
}

the error now is:

  Starting 80 tests (run ID: 2c51d59d-0cd2-40bd-88d8-bf64f74f8a15)
      fail [     35ms] anchor-centroid
           error: plugin errored with: Semantic(None, "invalid type: integer `1`, expected array")
               ┌─ /Users/rik/git/cetz/src/bezier.typ:399:21
               │
           399 │   let decoded = cbor(cetz-core.cubic_extrema_func(encoded))
               │                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

           help: error occurred in this call of function `cubic-extrema`
              ┌─ /Users/rik/git/cetz/src/path-util.typ:43:16
              │
           43 │       bounds += bezier.cubic-extrema(..pts)

and it's now possible to do assertions too. For example, in cubic_extrema in lib.rs when adding:

    if true {
        return Err("This is some error".to_string());
    }

The output is:

  Starting 80 tests (run ID: 10753ae1-1db3-4b27-9a08-85dd7f9ab268)
      fail [     34ms] anchor-centroid
           error: plugin errored with: This is some error
               ┌─ /Users/rik/git/cetz/src/bezier.typ:399:21
               │
           399 │   let decoded = cbor(cetz-core.cubic_extrema_func(encoded))
               │                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

           help: error occurred in this call of function `cubic-extrema`
              ┌─ /Users/rik/git/cetz/src/path-util.typ:43:16
              │
           43 │       bounds += bezier.cubic-extrema(..pts)

@rikhuijzer rikhuijzer mentioned this pull request Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant