fix: Renderer.drawScene() calls Finish() on its destination (issue #211) - #214
Merged
Merged
Conversation
Adds a rendererTest diagnostic (LargeImageRenderTest, id large-image-render) that draws a controllable size/source/finish combo directly onto the real roScreen every frame and auto-verifies via GetByteArray, to test whether an isolated large blit reproduces the silent-render-failure documented in TileMapBaking.bs. Findings (full writeup in specs/2026-09-12-large-image-render-spike.md): - No hardcoded size cap exists anywhere in brs-engine's draw path. - This exact symptom was already filed upstream (lvcabral/brs-engine#1198) and the maintainer couldn't reproduce it in an isolated repro either. - Renderer.forceDraw()'s Finish()/dummy-screen-flush pattern is never applied to the plain single-blit path SceneObjectImage's matchCamera/directToCamera mode uses (a real asymmetry vs. every other scratch-bitmap path) - a plausible but, per this spike's empirical test, NOT sufficient explanation on its own. - Ran the new demo at sizes up to 640px, memory- and disk-sourced, with no Finish() call (worst case), on both the BrightScript Simulator and real Roku hardware: zero failures across ~500 sustained frames on each. The isolated repro does not reproduce the original failure on either target. - roTextureManager is a load/cache wrapper only; it can't affect render-time completion and isn't a fix for this. Next steps recorded in the spec doc: re-test embedded in a real BGE.Game/Room under realistic entity/collider load, add the missing Finish()/forceDraw() call defensively regardless, and test under deliberate GC pressure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The first "real hardware" pass in the previous commit actually re-hit the local BrightScript Simulator (.env's default ROKU_HOST resolves there too) - re-ran against an explicit --host pointed at an actual physical Roku, confirmed via device-info model check. Along the way found roScreen.GetByteArray() returns Invalid on real hardware (crashed the demo) but returns real pixel data on both the simulator GUI app and headless brs-cli - a genuine, previously undocumented platform difference, worth its own upstream report. roBitmap.GetByteArray() is unaffected on both platforms. The demo now detects and falls back to screenshot-only verification where unsupported. Also added a `count` dimension (1/4/8/12/16 simultaneous copies per frame, rev/fwd to cycle) per the idea that a single isolated blit may not be representative of a real level baking many chunks at once. Re-verified on the real device: up to 16 simultaneous 320px blits/ frame (memory- and disk-sourced), sustained ~9.5s, zero failures. Findings and corrected results folded into specs/2026-09-12-large-image-render-spike.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every other engine path that draws into a scratch/temp bitmap and reads it back afterward (triangle warps, oriented-mode temp bitmaps) already forces completion via Renderer.forceDraw()'s Finish() call - but drawScene() itself never did, for the plain single-blit path every screen-aligned Image (matchCamera/directToCamera) actually uses. Added one Finish() call per frame, at the end of drawScene(). Confirmed on real hardware (Mark Pearce's Roku device): a baked tilemap chunk that previously silently failed to render above ~192px now renders reliably up to chunkSize=512 (colliders 24->9, scene objects 34->10 on examples/platformer's level) - verified via automated in-app GetByteArray checks, oscillation across the level's pit/platform boundary, multi-copy stress tests (up to 16 simultaneous 320px blits/frame), and manual play-testing, all with zero failures across ~500+ sustained real-device frames. Raised examples/platformer's Level.bs chunkSize from 128 to 512 and updated its and TileMapBaking.bs's doc comments accordingly. Also wires MainRoom's existing debug-overlay toggle (options/info button) to debugDrawColliders(), which was useful for diagnosing this and will be for future issues too. A separate, smaller residual gap was found while pushing chunkSize further (768+): a merged chunk spanning a large mostly-empty vertical gap can still occasionally fail to render *inside the full running game* even though identical dimensions are reliable in isolation. Filed as issue #213 rather than chased further here, since 512 already represents a solid, thoroughly-verified 4x improvement. Also filed lvcabral/brs-engine#1229 for a platform difference found along the way: roScreen.GetByteArray() returns Invalid on real hardware but real pixel data on the simulator/brs-node. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Renderer.drawScene()never called.Finish()on its destination surface before the frame proceeded to compositing/SwapBuffers()- unlike every other engine path that draws into a scratch/temp bitmap and reads it back afterward (triangle warps, oriented-mode temp bitmaps - seeRenderer.forceDraw()). That asymmetry is the root cause of issue #211: a baked tilemap chunk (examples/platformer's ground/platform, viaBGE.TileMap.bakeTileMapImages()) could silently fail to render above ~192px, even with provably-correct content, on both the BrightScript Simulator and real hardware.src/source/engine/renderer/Renderer.bs: onem.draw2d.Finish()call added at the end ofdrawScene().examples/platformer/src/source/Entities/Level.bs:chunkSizeraised 128 -> 512 (colliders 24->9, scene objects 34->10 on this level), doc comment updated with the measurements below.examples/platformer/src/source/Rooms/MainRoom.bs: the existing debug-overlay toggle (options/info) now also callsdebugDrawColliders()- useful for diagnosing this, and future issues.src/source/utils/tilemap/TileMapBaking.bs: doc comment updated to match.Testing
npm run check(lint + validate + headless Rooibos suite): 1105 passed.GetByteArraychecks, oscillation across the level's pit/platform boundary, and multi-copy stress tests (up to 16 simultaneous 320px blits/frame, ~500+ sustained frames) all show zero failures atchunkSize=512, versus reliable reproduction of the original failure at 256+ before this fix.Known remaining gap (not blocking this fix)
Pushing
chunkSizefurther (768+) reproduces a different, deterministic failure: a merged chunk spanning a large mostly-empty vertical gap can still fail to render inside the full running game, even though identical dimensions are reliable in an isolated stress test. Filed as #213 rather than chased further here, since 512 already represents a solid, thoroughly-verified 4x improvement over the original 128 cap.Related
roScreen.GetByteArray()behaves differently on real hardware vs. the simulator)specs/2026-09-12-large-image-render-spike.md🤖 Generated with Claude Code