Improve the drawing API #29
Conversation
WalkthroughThis set of changes introduces a new drawing module ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExampleCode
participant PenModule
participant NativePenFunctions
participant Graphics2D
User->>ExampleCode: Run drawing function
ExampleCode->>PenModule: Pen::drawLine(ctx, x1, y1, x2, y2)
PenModule->>NativePenFunctions: pic_nat_draw_line(ctx, x1, y1, x2, y2)
NativePenFunctions->>Graphics2D: drawLine(x1, y1, x2, y2)
Graphics2D-->>NativePenFunctions: (draws line)
NativePenFunctions-->>PenModule: return ctx
PenModule-->>ExampleCode: return ctx
sequenceDiagram
participant User
participant ExampleCode
participant ContextModule
participant NativeContextFunctions
participant Graphics2D
User->>ExampleCode: Request context from rect
ExampleCode->>ContextModule: Context::fromRect(ctx, x, y, w, h)
ContextModule->>NativeContextFunctions: pic_nat_gfx_from_rect(ctx, x, y, w, h)
NativeContextFunctions->>Graphics2D: create(x, y, w, h)
Graphics2D-->>NativeContextFunctions: returns new Graphics2D context
NativeContextFunctions-->>ContextModule: return new ctx
ContextModule-->>ExampleCode: return new ctx
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
examples/starrect.pics (1)
16-19: Optional: simplify rectangle rendering via Pen::drawRectUse the native rectangle primitive for clarity and potential performance.
- |> Pen::drawLine(x, y, x + w, y) - |> Pen::drawLine(x, y + h, x + w, y + h) - |> Pen::drawLine(x, y, x, y + w) - |> Pen::drawLine(x + w, y, x + w, y + h) + |> Pen::drawRect(x, y, w, h)piccode/render/context.pics (1)
12-12: Doc nit: “an handler” ➜ “a handle”Minor grammar fix in the return docs.
- // - (Reference) an handler to the Graphics2D. + // - (Reference) a handle to the Graphics2D.piccode/context/ctx.pics (2)
17-18: Doc accuracy: clarify return value for fromRectThese functions create derived contexts, not “rendered element” results.
- // Returns: - // - (Reference) A modified context with the rendered element. + // Returns: + // - (Reference) A new graphics sub-context derived from the given context.
27-28: Doc accuracy: clarify return value for fromSame wording improvement as fromRect.
- // Returns: - // - (Reference) A modified context with the rendered element. + // Returns: + // - (Reference) A new graphics context derived from the given context.piccode/pen/draw.pics (3)
19-20: Doc nit: “rect” ➜ “rectangle”Minor clarity improvement.
- // Draws a rect inside the specified context, based on the provided coordinates + // Draws a rectangle inside the specified context, based on the provided coordinates
33-34: Doc nit: specify “rounded rectangle”Clarify shape name.
- // Draws a rect inside the specified context, based on the provided coordinates + // Draws a rounded rectangle inside the specified context, based on the provided coordinates
49-50: Doc nit: specify “oval (ellipse)”Clarify shape name.
- // Draws a rect inside the specified context, based on the provided coordinates + // Draws an oval (ellipse) inside the specified context, based on the provided coordinatessrc/main/java/org/editor/nativemods/PiccodePenModule.java (1)
52-55: Silent precision loss when casting doubles to intsCoordinates are truncated with
(int)(double); fractional values are silently lost and large doubles can overflowint. Consider usingMath.roundor validating the range to keep behaviour predictable.src/main/java/org/editor/nativemods/PiccodeGfxModule.java (1)
31-60: Duplicate verbose type checks → extract helperThe four sequential
verifyTypecalls repeat in every native. Extract a small utility (e.g.,NativeChecks.expectNumber(caller, "x", x)) to cut boilerplate and reduce copy-paste errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/starrect.pics(1 hunks)piccode/context/ctx.pics(1 hunks)piccode/pen/draw.pics(1 hunks)piccode/render/context.pics(1 hunks)src/main/java/org/editor/menu/Menus.java(1 hunks)src/main/java/org/editor/nativemods/PiccodeGfxModule.java(2 hunks)src/main/java/org/editor/nativemods/PiccodePenModule.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-06T23:34:19.770Z
Learnt from: hexaredecimal
PR: Glimmr-Lang/PicassoCode#26
File: tests/test_readme_validation.py:233-236
Timestamp: 2025-08-06T23:34:19.770Z
Learning: The PicassoCode project README has specific formatting requirements including: app icon at "./src/main/resources/applogo/appicon.png" with 40% width, repository URL "gitgithub.com:hexaredecimal/Piccode.git", JAR file "Piccode-1.0-SNAPSHOT-jar-with-dependencies.jar", and elaborate ASCII art with "Creativity + Logic + Math" tagline in the license section.
Applied to files:
src/main/java/org/editor/nativemods/PiccodePenModule.java
🧬 Code Graph Analysis (2)
src/main/java/org/editor/nativemods/PiccodePenModule.java (2)
src/main/java/org/editor/CanvasFrame.java (2)
CanvasFrame(41-333)Override(105-127)src/main/java/org/piccode/piccode/Piccode.java (3)
initializeNativeAppModules(23-25)main(15-21)Piccode(13-26)
src/main/java/org/editor/nativemods/PiccodeGfxModule.java (1)
src/main/java/org/piccode/piccode/Piccode.java (1)
initializeNativeAppModules(23-25)
🔇 Additional comments (7)
src/main/java/org/editor/menu/Menus.java (1)
121-121: Typo fix in menu label looks good"Navigate" label is correct and aligns with the rest of the UI.
examples/starrect.pics (5)
5-5: Switched to Pen module import — OKThe example now depends on the new Pen API as intended.
9-12: Migration to Pen::drawLine is correctChained usage preserves the pipe style and returns ctx as expected.
23-25: OK: depth lines migrated to PenCoordinates preserved; chaining remains consistent.
29-30: OK: back lines migrated to PenAll good.
5-5: No remaining Render::drawLine usages foundAll searches for
Render::drawLinereturned zero matches, andpiccode/renderno longer exportsdrawLine. The newPen::drawLineentry points are in place:
- examples/starrect.pics:5 Pen :: import piccode.pen
- piccode/pen/draw.pics:2 Pen :: module { … }
- src/main/java/org/editor/nativemods/PiccodePenModule.java:23 public class PiccodePenModule
Nothing else needs to be migrated.
piccode/pen/draw.pics (1)
16-16: Pen::drawLine binding looks correctSignature and return value align with piping semantics.
| |> Pen::drawLine(x, y, x + w, y) | ||
| |> Pen::drawLine(x, y + h, x + w, y + h) | ||
| |> Pen::drawLine(x, y, x, y + w) | ||
| |> Pen::drawLine(x + w, y, x + w, y + h) |
There was a problem hiding this comment.
Bug: drawRect uses y + w instead of y + h for the left side
This draws a too-tall left edge when w != h.
Apply this minimal fix:
- |> Pen::drawLine(x, y, x, y + w)
+ |> Pen::drawLine(x, y, x, y + h)📝 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.
| |> Pen::drawLine(x, y, x + w, y) | |
| |> Pen::drawLine(x, y + h, x + w, y + h) | |
| |> Pen::drawLine(x, y, x, y + w) | |
| |> Pen::drawLine(x + w, y, x + w, y + h) | |
| |> Pen::drawLine(x, y, x + w, y) | |
| |> Pen::drawLine(x, y + h, x + w, y + h) | |
| - |> Pen::drawLine(x, y, x, y + w) | |
| + |> Pen::drawLine(x, y, x, y + h) | |
| |> Pen::drawLine(x + w, y, x + w, y + h) |
🤖 Prompt for AI Agents
In examples/starrect.pics around lines 16 to 19, the drawRect function
incorrectly uses y + w instead of y + h for the left side line, causing the left
edge to be too tall when width and height differ. Change the third drawLine call
to use y + h instead of y + w to correctly draw the rectangle's left edge.
| // Returns: | ||
| // - (Reference) A modified context with the rendered element. | ||
| from:: (ctx) = pic_nat_from_rect(ctx) | ||
|
|
There was a problem hiding this comment.
Critical: wrong native call and syntax in Context.from
Should be from :: (ctx) (space) and call the matching native pic_nat_from, not pic_nat_from_rect.
- from:: (ctx) = pic_nat_from_rect(ctx)
+ from :: (ctx) = pic_nat_from(ctx)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In piccode/context/ctx.pics at line 29, the Context.from function uses the wrong
native call and incorrect syntax. Change the function signature to use a space
as in `from :: (ctx)` and replace the native call from `pic_nat_from_rect` to
the correct `pic_nat_from`.
| public class PiccodePenModule { | ||
|
|
||
| public static void addFunctions() { | ||
|
|
There was a problem hiding this comment.
Module never registered → functions unusable
PiccodePenModule.addFunctions() is not wired into Piccode.initializeNativeAppModules(), so the new draw_* natives will never be exposed. Add the registration call next to PiccodeGfxModule::addFunctions or they will fail at runtime.
🤖 Prompt for AI Agents
In src/main/java/org/editor/nativemods/PiccodePenModule.java around lines 23 to
26, the addFunctions method is defined but never registered in
Piccode.initializeNativeAppModules(), causing the draw_* native functions to be
unavailable. To fix this, locate the Piccode.initializeNativeAppModules() method
and add a call to PiccodePenModule.addFunctions() alongside the existing
PiccodeGfxModule.addFunctions() call to ensure the new natives are properly
exposed at runtime.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
piccode/context/ctx.pics(1 hunks)piccode/pen/draw.pics(1 hunks)piccode/render/context.pics(1 hunks)src/main/java/org/editor/nativemods/PiccodeGfxModule.java(2 hunks)src/main/java/org/piccode/piccode/Piccode.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- piccode/render/context.pics
- piccode/pen/draw.pics
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/org/piccode/piccode/Piccode.java (1)
src/main/java/org/editor/nativemods/PiccodePenModule.java (1)
PiccodePenModule(23-170)
🔇 Additional comments (4)
src/main/java/org/piccode/piccode/Piccode.java (1)
6-6: Pen native module correctly registeredImport and registration look correct and keep initialization order sensible (Gfx first, then Pen). No issues.
Also applies to: 26-26
piccode/context/ctx.pics (2)
18-18: fromRect maps correctly to nativeNative call name aligns with Java side: gfx_from_rect → pic_nat_gfx_from_rect. Good.
28-28: from mapping and syntax fixedSignature
from :: (ctx)and nativepic_nat_gfx_fromare consistent with backend. Looks good.src/main/java/org/editor/nativemods/PiccodeGfxModule.java (1)
64-83: LGTM: gfx_from creates a proper cloneType checks, deref, and
gfx.create()usage are correct. Return asPiccodeReferencealigns with the new reference semantics.
| // | ||
| // Returns: | ||
| // - (Unit) Nothing | ||
| drop :: (ctx) = pic_nat_gfx_(ctx) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: wrong native call in drop()
drop :: (ctx) = pic_nat_gfx_(ctx) is missing the function suffix. It should call gfx_drop.
Apply this fix:
- drop :: (ctx) = pic_nat_gfx_(ctx)
+ drop :: (ctx) = pic_nat_gfx_drop(ctx)Run to verify symbol consistency:
I can push a commit to fix this and add a quick example using drop() if you want.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Pic scripts referencing pic_nat_gfx_*:"
rg -n "pic_nat_gfx_" -- piccode || true
echo "Native functions declared (Java):"
rg -n 'NativeFunctionFactory\.create\("gfx_[a-z_]+"' -- src/main/java || true
echo "Check for missing pic_nat_gfx_drop references:"
rg -n "pic_nat_gfx_drop" -- piccode || trueLength of output: 1141
Critical: fix native drop call in ctx.pics
The drop binding is calling a non-existent native symbol (pic_nat_gfx_). It needs the drop suffix to match the Java declaration (gfx_drop).
Location: piccode/context/ctx.pics:38
- drop :: (ctx) = pic_nat_gfx_(ctx)
+ drop :: (ctx) = pic_nat_gfx_drop(ctx)This aligns with the native function created in PiccodeGfxModule.java.
📝 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.
| drop :: (ctx) = pic_nat_gfx_(ctx) | |
| drop :: (ctx) = pic_nat_gfx_drop(ctx) |
🤖 Prompt for AI Agents
In piccode/context/ctx.pics at line 38, the drop binding incorrectly calls the
native symbol pic_nat_gfx_ which does not exist. Update the call to include the
drop suffix, changing it to pic_nat_gfx_drop to match the native Java
declaration in PiccodeGfxModule.java.
| NativeFunctionFactory.create("get_gfx", List.of(), (args, namedArgs, frame) -> { | ||
| var gfx = CanvasFrame.gfx; | ||
| var obj = Context.getObject(gfx.hashCode()); | ||
| if (obj == null) { | ||
| Context.allocate(gfx); | ||
| return makeObj(gfx); | ||
| } | ||
| return makeObj(gfx); | ||
| return new PiccodeReference(gfx); | ||
| }, null); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard get_gfx against null and surface a clear error
If CanvasFrame.gfx is null, returning a reference will fail later with a confusing type error. Retrieve caller info and throw a clear message when uninitialized.
Proposed change:
NativeFunctionFactory.create("get_gfx", List.of(), (args, namedArgs, frame) -> {
- var gfx = CanvasFrame.gfx;
- return new PiccodeReference(gfx);
+ var gfx = CanvasFrame.gfx;
+ var ctx = frame == null ? Context.top : Context.getContextAt(frame);
+ var caller = ctx.getTopFrame().caller;
+ if (gfx == null) {
+ throw new PiccodeException(caller.file, caller.line, caller.column,
+ "Graphics is not initialized (CanvasFrame.gfx is null)");
+ }
+ return new PiccodeReference(gfx);
}, null);I need to confirm that the helper classes and methods used in the proposed guard (Context, getContextAt, getTopFrame, caller, and the PiccodeException constructor signature) actually exist and match this usage. Let me verify their definitions.
🏁 Script executed:
#!/bin/bash
# Locate CanvasFrame and its gfx field
rg -n "class CanvasFrame" || echo "CanvasFrame class not found"
rg -n "gfx" src
# Check for a Context class and its APIs
rg -n "class Context" || echo "Context class not found"
rg -n "getContextAt" -n src || echo "getContextAt not found"
rg -n "top" -n src || echo "Context\\.top not found"
rg -n "getTopFrame" -n src || echo "getTopFrame not found"
rg -n "caller" -n src || echo "caller field not found"
# Inspect PiccodeException constructors
rg -n "class PiccodeException" -n src || echo "PiccodeException not found"
rg -n "new PiccodeException" -n srcLength of output: 14658
Guard get_gfx against uninitialized CanvasFrame.gfx
Add the same context–caller lookup and throw a clear PiccodeException when CanvasFrame.gfx is null, mirroring the other native gfx APIs.
• File: src/main/java/org/editor/nativemods/PiccodeGfxModule.java
• Lines: 26–29
NativeFunctionFactory.create("get_gfx", List.of(), (args, namedArgs, frame) -> {
- var gfx = CanvasFrame.gfx;
- return new PiccodeReference(gfx);
+ var gfx = CanvasFrame.gfx;
+ // derive caller location for proper error reporting
+ var ctx = frame == null ? Context.top : Context.getContextAt(frame);
+ var caller = ctx.getTopFrame().caller;
+ if (gfx == null) {
+ throw new PiccodeException(
+ caller.file, caller.line, caller.column,
+ "Graphics is not initialized (CanvasFrame.gfx is null)"
+ );
+ }
+ return new PiccodeReference(gfx);
}, null);📝 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.
| NativeFunctionFactory.create("get_gfx", List.of(), (args, namedArgs, frame) -> { | |
| var gfx = CanvasFrame.gfx; | |
| var obj = Context.getObject(gfx.hashCode()); | |
| if (obj == null) { | |
| Context.allocate(gfx); | |
| return makeObj(gfx); | |
| } | |
| return makeObj(gfx); | |
| return new PiccodeReference(gfx); | |
| }, null); | |
| NativeFunctionFactory.create("get_gfx", List.of(), (args, namedArgs, frame) -> { | |
| var gfx = CanvasFrame.gfx; | |
| // derive caller location for proper error reporting | |
| var ctx = frame == null ? Context.top : Context.getContextAt(frame); | |
| var caller = ctx.getTopFrame().caller; | |
| if (gfx == null) { | |
| throw new PiccodeException( | |
| caller.file, caller.line, caller.column, | |
| "Graphics is not initialized (CanvasFrame.gfx is null)" | |
| ); | |
| } | |
| return new PiccodeReference(gfx); | |
| }, null); |
🤖 Prompt for AI Agents
In src/main/java/org/editor/nativemods/PiccodeGfxModule.java lines 26 to 29, the
get_gfx native function does not check if CanvasFrame.gfx is null before
returning it. Add a guard to check if CanvasFrame.gfx is null, and if so,
retrieve the caller context and throw a PiccodeException with a clear error
message indicating that the graphics context is uninitialized. This should match
the error handling pattern used in other native gfx APIs.
| NativeFunctionFactory.create("gfx_from_rect", List.of("ctx", "x", "y", "w", "h"), (args, namedArgs, frame) -> { | ||
| var _ctx = namedArgs.get("ctx"); | ||
| var _x1 = namedArgs.get("x1"); | ||
| var _y1 = namedArgs.get("y1"); | ||
| var _x2 = namedArgs.get("x2"); | ||
| var _y2 = namedArgs.get("y2"); | ||
|
|
||
| var ctx = frame == null ? | ||
| Context.top | ||
| : Context.getContextAt(frame); | ||
| var x = namedArgs.get("x"); | ||
| var y = namedArgs.get("y"); | ||
| var w = namedArgs.get("w"); | ||
| var h = namedArgs.get("h"); | ||
|
|
||
| var ctx = frame == null | ||
| ? Context.top | ||
| : Context.getContextAt(frame); | ||
| var caller = ctx.getTopFrame().caller; | ||
|
|
||
| PiccodeValue.verifyType(caller, _ctx, Type.OBJECT); | ||
| PiccodeValue.verifyType(caller, _x1, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, _y1, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, _x2, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, _y2, Type.NUMBER); | ||
|
|
||
| var obj = (PiccodeObject) _ctx; | ||
| var map = obj.obj; | ||
| if (!map.containsKey("hash")) { | ||
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not an object"); | ||
| } | ||
|
|
||
| var _hash = map.get("hash"); | ||
| PiccodeValue.verifyType(caller, _hash, Type.NUMBER); | ||
| var hash = (int) (double) ((PiccodeNumber) _hash).raw(); | ||
| var _gfx = Context.getObject(hash); | ||
| if (_gfx == null) { | ||
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not allocated"); | ||
|
|
||
| PiccodeValue.verifyType(caller, _ctx, Type.REFERENCE); | ||
| PiccodeValue.verifyType(caller, x, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, y, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, w, Type.NUMBER); | ||
| PiccodeValue.verifyType(caller, h, Type.NUMBER); | ||
|
|
||
| var obj = (PiccodeReference) _ctx; | ||
| var _gfx = obj.deref(); | ||
| if (!(_gfx instanceof Graphics2D)) { | ||
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D"); | ||
| } | ||
|
|
||
| var gfx = (Graphics2D) _gfx; | ||
| var _x = (int) (double) ((PiccodeNumber) x).raw(); | ||
| var _y = (int) (double) ((PiccodeNumber) y).raw(); | ||
| var _w = (int) (double) ((PiccodeNumber) w).raw(); | ||
| var _h = (int) (double) ((PiccodeNumber) h).raw(); | ||
|
|
||
| var gfx2 = (Graphics2D) gfx.create(_x, _y, _w, _h); | ||
| return new PiccodeReference(gfx2); | ||
| }, null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate rectangle dimensions before creating sub-contexts
Negative or zero width/height can cause runtime errors or undefined behavior in Graphics2D#create(x, y, w, h). Validate early and provide a precise error.
Apply after the numeric casts:
var _x = (int) (double) ((PiccodeNumber) x).raw();
var _y = (int) (double) ((PiccodeNumber) y).raw();
var _w = (int) (double) ((PiccodeNumber) w).raw();
var _h = (int) (double) ((PiccodeNumber) h).raw();
+if (_w <= 0 || _h <= 0) {
+ throw new PiccodeException(caller.file, caller.line, caller.column,
+ "Width and height must be positive. Got w=" + _w + ", h=" + _h);
+}
var gfx2 = (Graphics2D) gfx.create(_x, _y, _w, _h);
return new PiccodeReference(gfx2);📝 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.
| NativeFunctionFactory.create("gfx_from_rect", List.of("ctx", "x", "y", "w", "h"), (args, namedArgs, frame) -> { | |
| var _ctx = namedArgs.get("ctx"); | |
| var _x1 = namedArgs.get("x1"); | |
| var _y1 = namedArgs.get("y1"); | |
| var _x2 = namedArgs.get("x2"); | |
| var _y2 = namedArgs.get("y2"); | |
| var ctx = frame == null ? | |
| Context.top | |
| : Context.getContextAt(frame); | |
| var x = namedArgs.get("x"); | |
| var y = namedArgs.get("y"); | |
| var w = namedArgs.get("w"); | |
| var h = namedArgs.get("h"); | |
| var ctx = frame == null | |
| ? Context.top | |
| : Context.getContextAt(frame); | |
| var caller = ctx.getTopFrame().caller; | |
| PiccodeValue.verifyType(caller, _ctx, Type.OBJECT); | |
| PiccodeValue.verifyType(caller, _x1, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, _y1, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, _x2, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, _y2, Type.NUMBER); | |
| var obj = (PiccodeObject) _ctx; | |
| var map = obj.obj; | |
| if (!map.containsKey("hash")) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not an object"); | |
| } | |
| var _hash = map.get("hash"); | |
| PiccodeValue.verifyType(caller, _hash, Type.NUMBER); | |
| var hash = (int) (double) ((PiccodeNumber) _hash).raw(); | |
| var _gfx = Context.getObject(hash); | |
| if (_gfx == null) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not allocated"); | |
| PiccodeValue.verifyType(caller, _ctx, Type.REFERENCE); | |
| PiccodeValue.verifyType(caller, x, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, y, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, w, Type.NUMBER); | |
| PiccodeValue.verifyType(caller, h, Type.NUMBER); | |
| var obj = (PiccodeReference) _ctx; | |
| var _gfx = obj.deref(); | |
| if (!(_gfx instanceof Graphics2D)) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D"); | |
| } | |
| var gfx = (Graphics2D) _gfx; | |
| var _x = (int) (double) ((PiccodeNumber) x).raw(); | |
| var _y = (int) (double) ((PiccodeNumber) y).raw(); | |
| var _w = (int) (double) ((PiccodeNumber) w).raw(); | |
| var _h = (int) (double) ((PiccodeNumber) h).raw(); | |
| var gfx2 = (Graphics2D) gfx.create(_x, _y, _w, _h); | |
| return new PiccodeReference(gfx2); | |
| }, null); | |
| var _x = (int) (double) ((PiccodeNumber) x).raw(); | |
| var _y = (int) (double) ((PiccodeNumber) y).raw(); | |
| var _w = (int) (double) ((PiccodeNumber) w).raw(); | |
| var _h = (int) (double) ((PiccodeNumber) h).raw(); | |
| if (_w <= 0 || _h <= 0) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, | |
| "Width and height must be positive. Got w=" + _w + ", h=" + _h); | |
| } | |
| var gfx2 = (Graphics2D) gfx.create(_x, _y, _w, _h); | |
| return new PiccodeReference(gfx2); |
🤖 Prompt for AI Agents
In src/main/java/org/editor/nativemods/PiccodeGfxModule.java between lines 31
and 63, after casting width and height to integers, add validation to check if
_w and _h are greater than zero. If either is zero or negative, throw a
PiccodeException with a clear message indicating invalid rectangle dimensions.
This prevents runtime errors from passing invalid dimensions to
Graphics2D#create.
| NativeFunctionFactory.create("gfx_drop", List.of("ctx"), (args, namedArgs, frame) -> { | ||
| var _ctx = namedArgs.get("ctx"); | ||
|
|
||
| private static PiccodeValue makeObj(Graphics2D obj) { | ||
| var _obj = new HashMap<String, PiccodeValue>(); | ||
| _obj.put("hash", new PiccodeNumber(obj.hashCode())); | ||
| _obj.put("class", new PiccodeString(obj.getClass().getName())); | ||
| return new PiccodeObject(_obj); | ||
| var ctx = frame == null | ||
| ? Context.top | ||
| : Context.getContextAt(frame); | ||
| var caller = ctx.getTopFrame().caller; | ||
|
|
||
| PiccodeValue.verifyType(caller, _ctx, Type.REFERENCE); | ||
| var obj = (PiccodeReference) _ctx; | ||
| var _gfx = obj.deref(); | ||
| if (!(_gfx instanceof Graphics2D)) { | ||
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D"); | ||
| } | ||
|
|
||
| var gfx = (Graphics2D) _gfx; | ||
| gfx.dispose(); | ||
| return new PiccodeUnit(); | ||
| }, null); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prevent accidental disposal of the root graphics context
Calling gfx_drop on the root CanvasFrame.gfx can break rendering for the entire app. Guard against disposing the root and provide a targeted error.
Apply:
var obj = (PiccodeReference) _ctx;
var _gfx = obj.deref();
if (!(_gfx instanceof Graphics2D)) {
throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D");
}
var gfx = (Graphics2D) _gfx;
-gfx.dispose();
+if (gfx == CanvasFrame.gfx) {
+ throw new PiccodeException(caller.file, caller.line, caller.column,
+ "Cannot dispose the root graphics context. Only sub-contexts created via gfx_from/gfx_from_rect should be dropped.");
+}
+gfx.dispose();
return new PiccodeUnit();Optional: If you prefer silent no-op instead of error, replace the throw with return new PiccodeUnit();.
🏁 Script executed:
#!/bin/bash
# Locate the CanvasFrame class definition
rg -n "class CanvasFrame" -C 5 src/main/java
# Find where a Graphics2D gfx field is declared
rg -n "Graphics2D\s.*gfx" -C 5 src/main/java
# Check for any direct references to CanvasFrame.gfx
rg -n "CanvasFrame\\.gfx" -n src/main/javaLength of output: 1772
Guard against disposing the root CanvasFrame.gfx
Before calling dispose() in gfx_drop, check if the target is the root context and error out:
File: src/main/java/org/editor/nativemods/PiccodeGfxModule.java
Lines: ~85–103
Apply:
var gfx = (Graphics2D) _gfx;
+ if (gfx == CanvasFrame.gfx) {
+ throw new PiccodeException(caller.file, caller.line, caller.column,
+ "Cannot dispose the root graphics context. Only sub-contexts created via gfx_from/gfx_from_rect should be dropped.");
+ }
gfx.dispose();
return new PiccodeUnit();📝 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.
| NativeFunctionFactory.create("gfx_drop", List.of("ctx"), (args, namedArgs, frame) -> { | |
| var _ctx = namedArgs.get("ctx"); | |
| private static PiccodeValue makeObj(Graphics2D obj) { | |
| var _obj = new HashMap<String, PiccodeValue>(); | |
| _obj.put("hash", new PiccodeNumber(obj.hashCode())); | |
| _obj.put("class", new PiccodeString(obj.getClass().getName())); | |
| return new PiccodeObject(_obj); | |
| var ctx = frame == null | |
| ? Context.top | |
| : Context.getContextAt(frame); | |
| var caller = ctx.getTopFrame().caller; | |
| PiccodeValue.verifyType(caller, _ctx, Type.REFERENCE); | |
| var obj = (PiccodeReference) _ctx; | |
| var _gfx = obj.deref(); | |
| if (!(_gfx instanceof Graphics2D)) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D"); | |
| } | |
| var gfx = (Graphics2D) _gfx; | |
| gfx.dispose(); | |
| return new PiccodeUnit(); | |
| }, null); | |
| NativeFunctionFactory.create("gfx_drop", List.of("ctx"), (args, namedArgs, frame) -> { | |
| var _ctx = namedArgs.get("ctx"); | |
| var ctx = frame == null | |
| ? Context.top | |
| : Context.getContextAt(frame); | |
| var caller = ctx.getTopFrame().caller; | |
| PiccodeValue.verifyType(caller, _ctx, Type.REFERENCE); | |
| var obj = (PiccodeReference) _ctx; | |
| var _gfx = obj.deref(); | |
| if (!(_gfx instanceof Graphics2D)) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, "Context is not a correct object. Expected Graphics2D"); | |
| } | |
| var gfx = (Graphics2D) _gfx; | |
| if (gfx == CanvasFrame.gfx) { | |
| throw new PiccodeException(caller.file, caller.line, caller.column, | |
| "Cannot dispose the root graphics context. Only sub-contexts created via gfx_from/gfx_from_rect should be dropped."); | |
| } | |
| gfx.dispose(); | |
| return new PiccodeUnit(); | |
| }, null); |
🤖 Prompt for AI Agents
In src/main/java/org/editor/nativemods/PiccodeGfxModule.java around lines 85 to
103, before calling dispose() on the Graphics2D object in the gfx_drop function,
add a check to determine if the target Graphics2D is the root CanvasFrame.gfx.
If it is the root context, throw an error instead of disposing it to prevent
invalid disposal. Implement this guard by comparing the _gfx object with the
root Graphics2D instance and conditionally throwing a PiccodeException if they
match.
The drawing context is now a reference value that is managed by the interpreter. This helps with reasoning about the mutable state of the context when we draw into it. This change allowed me to find a new home for common draw functions and context methods. You can now create sub contexts and render to them directly and its all tied together.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores