drawings with mcvrender #20
Conversation
…ean tested for rand. Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
…unction with C based from mcvrender. Signed-off-by: Sandro Cantarella <sandro@509dc541-f930-4e46-87da-4482e9d9a7bb.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@03829bc9-740a-4ae1-9848-e7d8baa7c8ba.fritz.box>
WalkthroughReplaced Python/PIL drawing and blending code with mvcrender primitives, added memory management around image buffers, introduced a rotation-aware size helper, broadened a TypedDict field type, adjusted handler signatures to accept optional destinations, pinned mvcrender version, added/modified docstrings, and updated tests with method binding changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant ReImageHandler
participant Buffer as img_work_layer (persistent)
participant Base as img_base_layer
participant Renderer as mvcrender
Caller->>ReImageHandler: get_image_from_rrm(m_json, destinations?)
Note over ReImageHandler: Build/refresh base image (np array)
alt Base exists
ReImageHandler->>Base: delete old base
end
ReImageHandler->>Base: create new base from m_json
ReImageHandler->>ReImageHandler: ensure Buffer exists & matches Base shape
ReImageHandler->>Buffer: copy Base into Buffer (in-place)
ReImageHandler->>Renderer: draw zones/flags/walls onto Buffer
Note over ReImageHandler: Optional destinations handled (None or data)
ReImageHandler-->>Caller: PIL image from Buffer
Note over ReImageHandler: delete temps (e.g., source arrays)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
sca075
temporarily deployed
to
valetudo_map_parser_pypi
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
SCR/valetudo_map_parser/config/utils.py (2)
165-168: Guard binary_image when bytes_format is False.On first frame, last_image may be None; this line can raise. Only set from last_image if it exists (or skip).
Apply one of these minimal fixes:
- else: - self.shared.binary_image = pil_to_png_bytes(self.shared.last_image) + else: + if hasattr(self.shared, "last_image") and self.shared.last_image is not None: + self.shared.binary_image = pil_to_png_bytes(self.shared.last_image) + else: + # Nothing to serialize yet + self.shared.binary_image = None
188-198: Fix return shape in exception path (must return a tuple).The function’s signature returns (image_or_bytes, dict), but except returns a single value.
Apply this diff:
- return ( - self.shared.last_image if hasattr(self.shared, "last_image") else None - ) + return ( + self.shared.last_image if hasattr(self.shared, "last_image") else None, + {}, + )
🧹 Nitpick comments (6)
tests/refactored.py (1)
575-607: Fix thecenterstype annotation
draw_filled_circleiterates overcentersexpecting an iterable of(x, y)pairs (e.g., an(N, 2)NumPy array), but the signature advertises a singleTuple[int, int]. Updating the hint avoids misuse and helps static checkers/tests.A minimal fix:
- centers: Tuple[int, int], + centers: np.ndarray | Sequence[tuple[int, int]],(add
from typing import Sequenceat the top if it isn’t already imported).SCR/valetudo_map_parser/config/utils.py (1)
698-707: Optional: add type hints and clearer name.Improves readability and static checks.
-def pil_size_rotation(image_rotate, pil_img): - """Return the size of the image.""" +def pil_size_rotation(image_rotate: int, pil_img: Image.Image) -> tuple[int, int]: + """Return (width, height) adjusted for rotation (90/270 swap)."""SCR/valetudo_map_parser/config/drawable.py (3)
56-80: Blending heuristic change acknowledged.Sampling center pixel and flooding the whole run is fast but may mismatch backgrounds at edges. If artifacts appear, consider per-pixel or a small-kernel sampling just around region edges.
197-202: Triangle via polygon_u8: ensure bounds/typing.xs/ys already int32; recommend clipping to image bounds before draw to avoid C-ext bounds checks on large coordinates.
384-458: Zones: efficient mask + in-place blend; two tweaks.
- Use a single-channel uint8 mask if polygon_u8 supports it to halve allocation.
- Optionally update alpha channel for perceptual consistency on translucent overlays.
SCR/valetudo_map_parser/rand256_handler.py (1)
71-75: Signatures now accept optional destinations—document behavior.Good change. Consider clarifying in docstrings that None is supported and how it alters room extraction/labels.
Also applies to: 126-131
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
SCR/valetudo_map_parser/config/drawable.py(6 hunks)SCR/valetudo_map_parser/config/rand256_parser.py(6 hunks)SCR/valetudo_map_parser/config/shared.py(2 hunks)SCR/valetudo_map_parser/config/types.py(1 hunks)SCR/valetudo_map_parser/config/utils.py(3 hunks)SCR/valetudo_map_parser/hypfer_draw.py(0 hunks)SCR/valetudo_map_parser/hypfer_handler.py(2 hunks)SCR/valetudo_map_parser/rand256_handler.py(8 hunks)SCR/valetudo_map_parser/rooms_handler.py(1 hunks)pyproject.toml(1 hunks)tests/refactored.py(5 hunks)
💤 Files with no reviewable changes (1)
- SCR/valetudo_map_parser/hypfer_draw.py
🧰 Additional context used
🧬 Code graph analysis (5)
SCR/valetudo_map_parser/rand256_handler.py (4)
SCR/valetudo_map_parser/config/types.py (1)
Destinations(37-41)SCR/valetudo_map_parser/config/async_utils.py (2)
AsyncPIL(38-66)async_fromarray(42-44)SCR/valetudo_map_parser/hypfer_draw.py (1)
async_get_robot_in_room(379-542)SCR/valetudo_map_parser/config/utils.py (1)
async_copy_array(477-479)
SCR/valetudo_map_parser/config/shared.py (1)
SCR/valetudo_map_parser/config/utils.py (1)
pil_size_rotation(698-706)
SCR/valetudo_map_parser/config/drawable.py (1)
SCR/valetudo_map_parser/config/colors.py (1)
sample_and_blend_color(457-519)
tests/refactored.py (2)
SCR/valetudo_map_parser/hypfer_draw.py (1)
ImageDraw(19-573)SCR/valetudo_map_parser/config/drawable.py (1)
async_draw_obstacles(572-628)
SCR/valetudo_map_parser/hypfer_handler.py (1)
SCR/valetudo_map_parser/config/utils.py (1)
async_copy_array(477-479)
🔇 Additional comments (5)
SCR/valetudo_map_parser/hypfer_handler.py (1)
257-263: LGTM: targeted buffer cleanup reduces peak memory.Deleting stale base/work arrays before reallocation and freeing the temp source after copy is sound.
Also applies to: 276-279
SCR/valetudo_map_parser/config/rand256_parser.py (1)
1-2: Docstring updates look good.Clearer intent without changing behavior.
Also applies to: 82-89, 92-117, 120-137, 155-172, 218-283
SCR/valetudo_map_parser/config/utils.py (2)
114-121: Good: proactively closing previous last_image.Prevents file/memory leaks before reassigning last_image.
240-241: LGTM: rotation-aware size extraction.Centralizing size logic via pil_size_rotation is cleaner.
SCR/valetudo_map_parser/rand256_handler.py (1)
61-62: LGTM: persistent work buffer and explicit frees.Reusing img_work_layer with np.copyto and deleting stale base/temp arrays reduces allocations and peak RSS.
Also applies to: 163-177, 236-266
| Pillow = ">=10.3.0" | ||
| scipy = ">=1.12.0" | ||
| mvcrender = ">=0.0.5" | ||
| mvcrender = "==0.0.6" |
There was a problem hiding this comment.
Pin to a published mvcrender version
mvcrender==0.0.6 isn’t available on PyPI (latest published release is 0.0.5), so this pin will break installs/CI the moment Poetry resolves dependencies. Please stick to a published version (e.g. restore >=0.0.5 or pin to ==0.0.5) unless you also vendor or publish 0.0.6 in your index. Based on learnings
Apply this diff to restore the published requirement:
-mvcrender = "==0.0.6"
+mvcrender = ">=0.0.5"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mvcrender = "==0.0.6" | |
| mvcrender = ">=0.0.5" |
🤖 Prompt for AI Agents
In pyproject.toml around line 21, the dependency is pinned to mvcrender==0.0.6
which is not published on PyPI and will break installs/CI; change the
requirement to a published version such as restoring a range (e.g.
mvcrender>=0.0.5) or pinning to the latest published exact version
(mvcrender==0.0.5) so Poetry can resolve dependencies successfully, or
alternatively vendor or publish 0.0.6 to your index if that specific version is
required.
| from mvcrender.draw import circle_u8, line_u8, polygon_u8 | ||
| from PIL import Image, ImageDraw, ImageFont |
There was a problem hiding this comment.
🧩 Analysis chain
Verify mvcrender API availability or add guarded fallbacks.
Imports rely on mvcrender.blend and mvcrender.draw (circle_u8/line_u8/polygon_u8). PyPI shows 0.0.5 focused on AutoCrop; these modules may not exist and would crash at import. Suggest try/except ImportError with local fallbacks, or pin to a version that provides these symbols.
Based on learnings
🌐 Web query:
Does the mvcrender PyPI package provide draw primitives (circle_u8, line_u8, polygon_u8) and blend helpers (get_blended_color, sample_and_blend_color)? Which version introduced them?
💡 Result:
Short answer: No. The mvcrender package only exposes the AutoCrop utility in its public docs/repo; it does not provide circle_u8, line_u8, polygon_u8, get_blended_color or sample_and_blend_color (no release notes or source files show those APIs). See the PyPI project page and the GitHub repository README. [1][2]
Sources:
- mvcrender on PyPI (project description & release history). [1]
- mvcrender GitHub repository (source/README listing implemented functions). [2]
Guard or replace missing mvcrender APIs in drawable.py: The imports for circle_u8, line_u8, polygon_u8 (mvcrender.draw) and blend helpers aren’t present in any released mvcrender version, causing import errors. Add try/except fallbacks or vendor these functions, or remove the dependency.
🤖 Prompt for AI Agents
SCR/valetudo_map_parser/config/drawable.py around lines 18-19, the direct
imports of circle_u8, line_u8, polygon_u8 (and related blend helpers) from
mvcrender.draw are not available in released mvcrender and cause ImportError;
wrap the mvcrender imports in a try/except ImportError and provide safe
fallbacks: either vendor minimal local implementations of
circle_u8/line_u8/polygon_u8 and the blend helpers with matching signatures that
use Pillow primitives, or replace usages to call Pillow ImageDraw/putpixel
equivalents and remove mvcrender dependency; ensure the fallback functions are
covered by unit tests and update the module-level imports to import the fallback
when mvcrender is absent.
testing mcvrender vs pillow
Summary by CodeRabbit
Refactor
Bug Fixes
Chores