Skip to content

perf(map): eliminate redundant PNG serialization in Q10 rendering pipeline - #973

Open
allenporter wants to merge 1 commit into
Python-roborock:mainfrom
allenporter:perf-q10-render-pipeline
Open

allenporter wants to merge 1 commit into
Python-roborock:mainfrom
allenporter:perf-q10-render-pipeline

Conversation

@allenporter

@allenporter allenporter commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

In render_q10_map, the base packet was previously rendered to PNG bytes via parsed_from_packet. Then, whenever the map contained drawables (charger, trace path, virtual walls, or no-go zones—which is almost always true), _draw_map_content re-serialized the image to PNG bytes a second time, discarding the first PNG bytes.

This change:

  1. Adds B01Q10MapParser.map_data_from_packet(packet) returning MapData with the rendered base Image without performing premature PNG encoding.
  2. In render_q10_map, defers PNG serialization to the end of the pipeline so that images are encoded exactly once.
  3. Keeps parsed_from_packet fully backward-compatible for existing callers.
  4. Adds test_benchmark_q10_map_render_with_trace benchmark exercising the composite path.

Performance Impact

Eliminates duplicate PNG compression (Image.save(..., format="PNG")) and intermediate byte buffer allocations whenever overlays or traces are drawn:

  • test_benchmark_q10_map_render_with_trace: 0.804 ms $\rightarrow$ 0.257 ms (3.1x faster / 68% latency reduction)
  • Full composite render (200x200 grid + trace session at Scale 4): saves ~15.1 ms of redundant PNG compression time per frame.
Workload Before (#972 on main) After (This PR) Change
render_q10_map with Trace 0.804 ms 0.257 ms -0.547 ms (3.1x faster)
Redundant PNG Compression (800x800) ~15.1 ms 0.0 ms Eliminated

Note: The existing test_benchmark_q10_map_full_scale_render tests only base grid parsing via parse_packet, so their numbers are unchanged. The win is captured in composite rendering with overlays (render_q10_map).

Verification

  • All map tests pass (uv run pytest tests/map/).
  • Verified with test_benchmark_q10_map_render_with_trace image integrity check.

Copilot AI lite review requested due to automatic review settings September 28, 2026 03:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Benchmark coverage and percentile reporting need correction before approval.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

Optimizes Q10 map rendering by deferring PNG serialization until composition completes, while preserving existing parsing APIs.

Changes:

  • Adds PNG-free MapData rendering.
  • Serializes composed images once.
  • Optimizes room-color lookup.
  • Adds map benchmarks and profiling support.
File Summary
tests/​map/​test_benchmarks.py Adds map benchmarks; composite coverage does not exercise overlays, and the reported P95 calculation is incorrect.
roborock/​map/​room_colors.py Adds lookup-table-based room color resolution.
roborock/​map/​b01_q10_render.py Defers PNG serialization in the rendering pipeline.
roborock/​map/​b01_q10_map_parser.py Separates image construction from PNG encoding while preserving compatibility.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/map/test_benchmarks.py
Comment thread tests/map/test_benchmarks.py Outdated
hCoureau

This comment was marked as resolved.

@allenporter
allenporter force-pushed the perf-q10-render-pipeline branch 2 times, most recently from fc6c0fd to 7c59a0b Compare October 5, 2026 14:17
@allenporter
allenporter force-pushed the perf-q10-render-pipeline branch from 7c59a0b to 4a38ee6 Compare October 12, 2026 02:15
@allenporter
allenporter marked this pull request as ready for review October 13, 2026 12:59
@allenporter
allenporter force-pushed the perf-q10-render-pipeline branch from 4a38ee6 to 3607ff1 Compare October 13, 2026 13:06
@allenporter
allenporter marked this pull request as draft October 13, 2026 13:17
@allenporter
allenporter force-pushed the perf-q10-render-pipeline branch from 3607ff1 to a49913c Compare October 13, 2026 13:20
@allenporter
allenporter force-pushed the perf-q10-render-pipeline branch from a49913c to db2820c Compare October 13, 2026 13:24
@allenporter
allenporter marked this pull request as ready for review October 13, 2026 13:27
@allenporter
allenporter requested a review from Lash-L October 13, 2026 13:27

@allenporter allenporter left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A separate future change will remove some of the methods that still exist just for testing.

This branch has not been deployed

No deployments
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.

4 participants