Skip to content

Conversation

dbp
Copy link
Collaborator

@dbp dbp commented Aug 13, 2025

The origin for the line should be at x1, y1, but since the arguments to makeOverlayImage move the background right and down, we need to reverse both. This will mean that the background goes left and up and the start of the line is at the right place.

This fixes #1801.

The origin for the line should be at x1, y1, but since the arguments to
makeOverlayImage move the _background_ right and down, we need to
reverse both. This will mean that the background goes left and up and
the start of the line is at the right place.
@dbp dbp requested a review from blerner August 13, 2025 23:41
@dbp
Copy link
Collaborator Author

dbp commented Aug 14, 2025

Turns out this isn't good enough; still are counter-examples :)

@dbp dbp marked this pull request as draft August 14, 2025 00:19
As it turns out, if your line is in the "wrong direction", the previous
code didn't work. You have to shift by the closest point to the origin.
@dbp dbp marked this pull request as ready for review August 14, 2025 00:51
@dbp
Copy link
Collaborator Author

dbp commented Aug 14, 2025

My test cases were:

include image

c = circle(30, "outline", "maroon")

i1 = add-line(c, 0, 0, 30, 30, "red")
i2 = add-line(c, 0, 0, 30, 0, "red")
i3 = add-line(c, 0, 0, 0, 30, "red")


i4 = add-line(c, 0, 30, 30, 0, "red")
i5 = add-line(c, 30, 0, 0, 30, "red")
i6 = add-line(c, 30, 0, 30, 30, "red")

i7 = add-line(c, 30, 30, 60, 60, "red")
i8 = add-line(c, 30, 30, 60, 30, "red")
i9 = add-line(c, 30, 30, 30, 60, "red")

i10 = add-line(c, 30, 60, 60, 30, "red")
i11 = add-line(c, 60, 30, 30, 60, "red")


above-list([list: 
    beside-list([list: i1, i2, i3]),
    beside-list([list: i4, i5, i6]),
    beside-list([list: i7, i8, i9]),
    beside-list([list: i10, i11])])

Which renders as:

Screenshot 2025-08-13 at 8 54 51 PM

These were confirmed against the behavior of 2htdp/image (and also, manually inspected)

The only odd thing about this is not a problem with add-line, exactly -- if a line is against the edge of the image, it doesn't render. You can see this with i2 above. This same phenomenon shows up if you try to render just line(0, 30, "red"). But of course, once you place it on top of a background that gives it space, it shows up.

@jpolitz
Copy link
Member

jpolitz commented Aug 14, 2025

I think this works the way I expect. It causes a crash with both negative x/y values on the command-line node-canvas – like literally a V8-internal assertion failure. But other than that I can't find an image that doesn't look like what I expect...

[17:57:59] (horizon *1)→ cat line.arr
include image
sq = square(25, "outline", "gray")
row = beside-list([list: sq, sq, sq, sq, sq, sq, sq, sq, sq, sq])
grid = above-list([list: row, row, row, row, row, row, row, row, row, row])

save-image( add-line(grid, -12.5, -10, 25, 10, "darkgreen"), "green-line.png")

[17:52:41] (horizon *1)→ node line.jarr


#
# Fatal error in , line 0
# Check failed: static_cast<int64_t>(amount_before) >= -delta.
#
#
#
#FailureMessage Object: 0x16dc056a8
----- Native stack trace -----

 1: 0x1023806cc node::NodePlatform::GetStackTracePrinter()::$_0::__invoke() [/usr/local/bin/node]
 2: 0x103b1c3d8 V8_Fatal(char const*, ...) [/usr/local/bin/node]
 3: 0x10274117c v8::internal::Heap::OldArrayBufferBytes() [/usr/local/bin/node]
 4: 0x1024fe1e4 v8::Isolate::AdjustAmountOfExternalAllocatedMemory(long long) [/usr/local/bin/node]
 5: 0x1022a9a6c napi_adjust_external_memory [/usr/local/bin/node]

@jpolitz
Copy link
Member

jpolitz commented Aug 14, 2025

include image
sq = square(25, "outline", "gray")
row = beside-list([list: sq, sq, sq, sq, sq, sq, sq, sq, sq, sq])
grid = above-list([list: row, row, row, row, row, row, row, row, row, row])

save-image( add-line(grid, 0, 5, 25, 5, "red"), "red-line.png")
save-image( add-line(grid, 12.5, 10, 25, 10, "darkgreen"), "green-line.png")
save-image( add-line(grid, 0, 0, 25, 25, "purple"), "purple-line.png")
image image image

@blerner
Copy link
Member

blerner commented Aug 14, 2025

Hrm save-image shouldn't crash with negative coordinates; we should choose which of two semantics to impose here:

  • translate the rendered image so its bounding-box is all in Quadrant I
  • crop the image to Quadrant I

Images shouldn't expose the fact that their coordinates might be negative

@dbp
Copy link
Collaborator Author

dbp commented Aug 14, 2025

Hrm save-image shouldn't crash with negative coordinates; we should choose which of two semantics to impose here:

  • translate the rendered image so its bounding-box is all in Quadrant I
  • crop the image to Quadrant I

Images shouldn't expose the fact that their coordinates might be negative

Is this a problem that was created by add-line, or is this a general issue with how save-image currently works? (as in, is it a part of this PR, or should it be a separate issue)

@blerner
Copy link
Member

blerner commented Aug 14, 2025

It's probably a separate issue; we didn't even have save-image until recently, when we moved the image library into pyret-lang. But it's going to be exposed sooner than later in your usage, largely because of the way add-line might get used...

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.

Add-line seems to sometimes not render line at all.
3 participants