[panglesd]

This PR does two things:

• Improve drawing performance, in the following way:
  • Avoids resetting the d attribute if it has not changed. Just in case the browser would rerender the same thing.
  • Avoid recomputing the "full" d attribute when time is bigger than the end time of the stroke. Saves a lot of recomputation!
  • Do not wait for an animation frame in fast mode. Otherwise, many steps end up taking a lot of time!
• Fix a bug where Lwd-controlled DOM is not updated yet (it waits an animation frame) when eg coordinates are computed. Uses the new and fancy screenshot-based cram tests 🙂)

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 9c34f9a

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e71fc4e9-0d9d-45d6-93bb-15e38b97583f

Commits

Reviewing files that changed from the base of the PR and between 4798df3 and 9c34f9a.

Files ignored due to path filters: 4

• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-00-chromium-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-00-firefox-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-02-chromium-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-02-firefox-linux.png is excluded by !**/*.png

Files selected for processing: 1

• CHANGELOG.md

Files skipped from review as they are similar to previous changes: 1

• CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The drawing changes centralize stroke construction and path generation. Preview rendering uses stroke timing and a stored final path. Playback checks continuation before scheduling its first animation frame. The PR adds a screenshot fixture for a focused drawing and updates the Playwright test setup. In the vendored reactive DOM code, attachment updates compare values, and invalidation callbacks use microtasks instead of animation-frame callbacks.

Possibly related PRs

• panglesd/slipshow#286: Adds the Playwright screenshot-test workflow, runner, and test configuration. This PR changes that execution contract and adds a fixture that uses its test conventions.

Priority: Low

Merge Risk: Low · up to 9c34f

Drawing playback may omit a stroke’s final point, and an empty decoded path may fail during timing sampling. These narrow concerns warrant owner attention; the changelog wording is corrected.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "Improve drawing performance (and fix a bug)" matches the main change. The PR avoids resetting unchanged d attributes, skips recomputing full paths after a stroke ends, and drops the anima…
Description check		The description lists the three performance changes: skipping unchanged d resets, skipping full d recomputation after stroke end time, and removing the animation-frame wait in fast mode. It also d…
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "Improve drawing performance (and fix a bug)" matches the main change. The PR avoids resetting unchanged d attributes, skips recomputing full paths after a stroke ends, and drops the animation-frame wait in fast mode. The Lwd DOM timing fix is the bug fix named in the parenthetical. The title is concise and specific enough for a teammate scanning history.

Full details: Description check

Explanation

The description lists the three performance changes: skipping unchanged d resets, skipping full d recomputation after stroke end time, and removing the animation-frame wait in fast mode. It also describes the Lwd DOM timing bug fix and the screenshot-based cram tests. These items match the files in the raw summary, including preview.ml, path_editing.ml, elwd.ml, and the new test files.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c367456d-7582-4854-b4b0-4ada8eeb011c

Commits

Reviewing files that changed from the base of the PR and between e3eb081 and d4d5b35.

Files ignored due to path filters: 4

• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-00-chromium-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-00-firefox-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-02-chromium-linux.png is excluded by !**/*.png
• test/screenshot-based/drawing-focus-fast-mode/screenshots/presentation-step-02-firefox-linux.png is excluded by !**/*.png

Files selected for processing: 12

• src/engine/runtime/drawing/drawing_controller/preview.ml
• src/engine/runtime/drawing/drawing_controller/tools.ml
• src/engine/runtime/drawing/drawing_state/drawing_state.ml
• src/engine/runtime/drawing/drawing_state/json.ml
• src/engine/runtime/drawing/drawing_state/path_editing.ml
• src/engine/runtime/drawing/drawing_state/types.ml
• test/screenshot-based/drawing-focus-fast-mode/drawing.draw
• test/screenshot-based/drawing-focus-fast-mode/dune
• test/screenshot-based/drawing-focus-fast-mode/presentation.slp
• test/screenshot-based/drawing-focus-fast-mode/run.json
• test/screenshot-based/playwright-docker.sh
• vendor/github.com/panglesd/lwd/lib/brr-lwd/elwd.ml

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Add a fallback for browsers without queueMicrotask. · elwd.ml:474-477

vendor/github.com/panglesd/lwd/lib/brr-lwd/elwd.ml:474-477
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Add a fallback for browsers without queueMicrotask.

Jv.get Jv.global "queueMicrotask" returns undefined when the standalone HTML runs in a browser without queueMicrotask. The invalidation callbacks then pass that value to Jv.apply, so reactive DOM updates fail. Preserve queueMicrotask when available and use setTimeout otherwise.

Suggested fix

 let queue_microtask =
   let queue_microtask = Jv.get Jv.global "queueMicrotask" in
   fun f ->
-    ignore (Jv.apply queue_microtask [| Jv.callback ~arity:1 (fun _ -> f ()) |])
+    let callback = Jv.callback ~arity:1 (fun _ -> f ()) in
+    if Jv.is_undefined queue_microtask then
+      ignore (Jv.call Jv.global "setTimeout" [| callback; Jv.of_int 0 |])
+    else
+      ignore (Jv.apply queue_microtask [| callback |])

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: e155fae5-a867-4e06-81be-fab2ef8f60b8

Commits

Reviewing files that changed from the base of the PR and between 3cfc745 and 4798df3.

Files selected for processing: 1

• CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.