Skip to content

fix: resolve QPoint/QPointF type mismatch when moving vertices fixed #237#238

Closed
feyzhen wants to merge 1 commit into
vietanhdev:mainfrom
feyzhen:fix-points_over_window
Closed

fix: resolve QPoint/QPointF type mismatch when moving vertices fixed #237#238
feyzhen wants to merge 1 commit into
vietanhdev:mainfrom
feyzhen:fix-points_over_window

Conversation

@feyzhen

@feyzhen feyzhen commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

intersection_point() returns a QPoint, which cannot be subtracted from a QPointF in shape vertices, causing a TypeError on drag. Normalize all coordinates to QPointF before arithmetic, fixing the crash when dragging vertices or moving shapes.

intersection_point() returns a QPoint, which cannot be subtracted
from a QPointF in shape vertices, causing a TypeError on drag.
Normalize all coordinates to QPointF before arithmetic, fixing the
crash when dragging vertices or moving shapes.
@vietanhdev

Copy link
Copy Markdown
Owner

Hi @feyzhen — thanks for tracking this down and filing #237 with a clean repro.

While reviewing this I found #240, which turned out to fix the same underlying issue (intersection_point() returning QPoint instead of QPointF) but at the source rather than at each call site. I tested both fixes against all three crash paths that stem from this type mismatch:

  1. Drawing a new shape to the image edge (canvas.py:291shape.py lineTo)
  2. Dragging an existing vertex to the edge (bounded_move_vertexbug: during labeling, while points or shape over the canvas, program will broken #237, this PR)
  3. Dragging a whole selected shape to the edge (bounded_move_shapes — this PR)

#240 fixes 1 and 2 by fixing intersection_point() itself, but not 3 — bounded_move_shapes never calls intersection_point(), so its QPoint/QPointF mixing at lines 635/638 is a separate instance of the same underlying bug. Your fix here is the only one of the two that catches it.

I've merged #240 (2b093a5) to close paths 1 and 2. Would you mind rebasing this PR down to just the bounded_move_shapes hunk? The bounded_move_vertex change here is now redundant (already fixed at the source by #240), but the bounded_move_shapes fix is still the only patch for that crash and is correct as-is. Happy to merge as soon as it's rebased.

Thanks again for catching this.

@vietanhdev

Copy link
Copy Markdown
Owner

Follow-up: rather than waiting on a rebase, I pulled your `bounded_move_shapes` fix out into its own PR (#241) so it could land independently of the now-redundant `bounded_move_vertex` change — merged as d91d959, with a `Co-authored-by` trailer crediting you for the fix.

All three crash paths in this bug class (drawing at the edge, dragging a vertex, dragging a shape) are now fixed on `main` via #240 + #241. Closing this one out since both of its fixes are now covered — but thank you again for the original report (#237) and this fix, it saved real debugging time.

@vietanhdev vietanhdev closed this Jul 11, 2026
vietanhdev added a commit that referenced this pull request Jul 11, 2026
…242)

Copilot's review of #241 flagged that the left/top clamp still did
int(o1.x()) before wrapping in QPointF, so sub-pixel overflows (e.g.
-0.3) truncate to 0 and silently fail to correct the position.

Verifying that fix surfaced a second, independent bug in the same
lines: the right/bottom clamp corrects against pixmap.width()/height()
instead of (width() - 1)/(height() - 1), which is the bound
out_off_pixmap() actually enforces — so a shape overflowing the right
or bottom edge stayed uncorrected regardless of the int() truncation.

Both are pre-existing (predate #237/#238/#240/#241); this PR only
touches the four lines already modified by #241. Adds a regression
test covering both edges plus the original TypeError crash path.
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.

2 participants