Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the Next2D Player version from 2.7.0 to 2.8.0 and adds a fallback hit-testing mechanism for shapes without a graphics buffer.
- Version bumped to 2.8.0 in both package.json and console log
- Added fallback rectangle-based hit testing for shapes with empty graphics buffers
- Added 15 new test cases to validate the hit-testing behavior with various transformations and edge cases
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| package.json | Updated package version from 2.7.0 to 2.8.0 |
| src/index.ts | Updated console log version string from 2.7.0 to 2.8.0 |
| packages/display/src/Shape/usecase/ShapeHitTestUseCase.ts | Added fallback hit-testing logic for shapes without graphics buffer, creating a default rectangle path based on shape bounds |
| packages/display/src/Shape/usecase/ShapeHitTestUseCase.test.ts | Added comprehensive test coverage for the new fallback hit-testing behavior, including transformations, edge cases, and boundary conditions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hit_context.setTransform( | ||
| tMatrix[0], tMatrix[1], tMatrix[2], | ||
| tMatrix[3], tMatrix[4], tMatrix[5] | ||
| ); |
There was a problem hiding this comment.
The setTransform call is duplicated - it's already called on lines 40-43 before the conditional check. The second call on lines 51-54 is redundant and should be removed.
| hit_context.setTransform( | |
| tMatrix[0], tMatrix[1], tMatrix[2], | |
| tMatrix[3], tMatrix[4], tMatrix[5] | |
| ); |
| tMatrix[0], tMatrix[1], tMatrix[2], | ||
| tMatrix[3], tMatrix[4], tMatrix[5] | ||
| ); | ||
| hit_context.beginPath(); |
There was a problem hiding this comment.
The beginPath call is also duplicated - it's already called on line 39 before the conditional check. The second call on line 55 is redundant and should be removed.
| hit_context.beginPath(); |
| hit_context.moveTo(0, 0); | ||
| hit_context.lineTo(width, 0); | ||
| hit_context.lineTo(width, height); | ||
| hit_context.lineTo(0, height); | ||
| hit_context.lineTo(0, 0); |
There was a problem hiding this comment.
The rectangle path should start from (graphics.xMin, graphics.yMin) instead of (0, 0). The current implementation assumes the shape bounds start at the origin, but shapes can have non-zero xMin/yMin values (as tested in test case23 with negative coordinates). The path should be:
hit_context.moveTo(graphics.xMin, graphics.yMin);
hit_context.lineTo(graphics.xMax, graphics.yMin);
hit_context.lineTo(graphics.xMax, graphics.yMax);
hit_context.lineTo(graphics.xMin, graphics.yMax);
hit_context.lineTo(graphics.xMin, graphics.yMin);| hit_context.moveTo(0, 0); | |
| hit_context.lineTo(width, 0); | |
| hit_context.lineTo(width, height); | |
| hit_context.lineTo(0, height); | |
| hit_context.lineTo(0, 0); | |
| hit_context.moveTo(graphics.xMin, graphics.yMin); | |
| hit_context.lineTo(graphics.xMax, graphics.yMin); | |
| hit_context.lineTo(graphics.xMax, graphics.yMax); | |
| hit_context.lineTo(graphics.xMin, graphics.yMax); | |
| hit_context.lineTo(graphics.xMin, graphics.yMin); |
|
指摘箇所を改修するので、一旦close |
No description provided.