Skip to content

perf: extend viewport culling to updateCache and markStageDirty for off-screen blocks#7798

Closed
ssz2605 wants to merge 3 commits into
sugarlabs:masterfrom
ssz2605:perf/viewport-culling-cache-fix
Closed

perf: extend viewport culling to updateCache and markStageDirty for off-screen blocks#7798
ssz2605 wants to merge 3 commits into
sugarlabs:masterfrom
ssz2605:perf/viewport-culling-cache-fix

Conversation

@ssz2605

@ssz2605 ssz2605 commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

PR Category

  • Performance

Problem

During playback, every executed block is highlighted and then unhighlighted. Each transition performs a synchronous container.updateCache() and triggers markStageDirty(), resulting in a subsequent stage.update().

For large projects such as Crab Canon Plot, many blocks extend well beyond the visible viewport. However, the highlight logic only checked container.visible (the eye icon) and not whether the block was actually within the viewport. As a result:

  • Off-screen blocks still performed expensive updateCache() operations.
  • Off-screen highlight/unhighlight triggered unnecessary stage redraws.
  • Rendering work was performed for visual changes that were never visible to the user.

Solution

Skip rendering work for blocks outside the viewport while preserving correct highlight state.

Changes

  • js/block.js

    • Skip updateCache() during highlight() when the block is outside the viewport.
    • Skip updateCache() during unhighlight() for off-screen blocks while still updating bitmap state to prevent stale highlights when the block becomes visible again.
  • js/logo.js

    • Only call markStageDirty() after highlight() if the block is visible in the viewport.
    • Only call markStageDirty() after deferred unhighlight() if the block is visible.
    • Only call markStageDirty() after end-of-child-flow unhighlight() if the block is visible.

Results

Metric Before After
updateCache() calls 5,800 866 (-85%)
Time spent in updateCache() 600 ms 90 ms (-85%)
stage.update() (best case) 411 ms 369 ms (-10%)
stage.update() (worst case) 1,476 ms 1,239 ms (-16%)

Note

  • Bitmap visibility toggling in unhighlight() remains unconditional , only updateCache() is gated. This prevents stale highlight state when a block scrolls back into viewport.
  • This extends the viewport culling introduced in perf: cull off-screen blocks from display list to reduce stage.update() cost #7738 to the block highlight rendering path.
    -This does not affect total playback time, which is dominated by audio scheduling. The improvement is in rendering efficiency and frame consistency.

Verification

  • Lint: 0 errors
  • Prettier: clean
  • Tests: 187 suites, 6,585 tests passed

@github-actions github-actions Bot added performance Improves performance (load time, memory, rendering) tests Adds or updates test coverage size/S Small: 10-49 lines changed area/javascript Changes to JS source files labels Jul 12, 2026
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 31.25000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.26%. Comparing base (e58db21) to head (7382b7f).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
js/logo.js 22.22% 7 Missing ⚠️
js/block.js 60.00% 2 Missing ⚠️
js/blocks.js 0.00% 1 Missing ⚠️
js/turtle-singer.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7798      +/-   ##
==========================================
- Coverage   59.27%   59.26%   -0.01%     
==========================================
  Files         176      176              
  Lines       57975    57979       +4     
==========================================
- Hits        34365    34364       -1     
- Misses      23610    23615       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ssz2605 ssz2605 changed the title perf: skip block cache updates and stage renders for off-screen highlights perf: extend viewport culling to updateCache and markStageDirty for off-screen blocks Jul 12, 2026
@github-actions github-actions Bot added size/L Large: 250-499 lines changed and removed size/S Small: 10-49 lines changed labels Jul 13, 2026
@ssz2605 ssz2605 force-pushed the perf/viewport-culling-cache-fix branch from 2e0f0b6 to f56c038 Compare July 13, 2026 17:39
@github-actions github-actions Bot added size/M Medium: 50-249 lines changed and removed size/L Large: 250-499 lines changed labels Jul 13, 2026
@ssz2605 ssz2605 force-pushed the perf/viewport-culling-cache-fix branch 6 times, most recently from df16f72 to fb7add5 Compare July 13, 2026 20:01
ssz2605 added 2 commits July 14, 2026 01:57
Gate container.updateCache() in Block.unhighlight() and

markStageDirty() in logo.js highlight/unhighlight paths on

block._viewportVisible. highlight() early-returns for

off-screen blocks, skipping bitmap toggling and updateCache.

Builds on PR sugarlabs#7738 viewport culling by eliminating the

remaining per-frame overhead for off-screen blocks.

Measured improvement (Crabcanon-plot, 900 blocks):

- updateCache calls: ~5800 -> ~866 (85% reduction)

- updateCache time: ~600ms -> ~90ms
@ssz2605 ssz2605 force-pushed the perf/viewport-culling-cache-fix branch from fb7add5 to ba310f7 Compare July 13, 2026 20:28
@github-actions github-actions Bot added size/S Small: 10-49 lines changed and removed size/M Medium: 50-249 lines changed labels Jul 13, 2026
… invalid refs

- Fix viewport visibility checks to use Block objects instead of block IDs
  (blk is a number, not an object - blk._viewportVisible was always undefined)
- Guard unhighlight() against undefined block references (blocks.js:3409)
- Guard unhighlightQueue.pop() with null/undefined/blockList check (logo.js:2106)
@ssz2605 ssz2605 force-pushed the perf/viewport-culling-cache-fix branch from ba310f7 to 7382b7f Compare July 13, 2026 20:36
@github-actions github-actions Bot added size/M Medium: 50-249 lines changed and removed size/S Small: 10-49 lines changed labels Jul 13, 2026
@ssz2605 ssz2605 closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files performance Improves performance (load time, memory, rendering) size/M Medium: 50-249 lines changed tests Adds or updates test coverage

Projects

Development

Successfully merging this pull request may close these issues.

1 participant