-
Notifications
You must be signed in to change notification settings - Fork 0
feat: display project boundary, add panning and a bit of zoom #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
""" WalkthroughThis update introduces a camera system with panning and zooming capabilities, affecting pointer handling, rendering logic, and UI scaling. The render scale is now dynamically updated based on camera zoom and canvas size, with corresponding changes in Zig rendering code to scale line widths and UI elements. A project boundary is also visually rendered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Canvas
participant PointerController
participant Camera
participant ZigLogic
User->>Canvas: Mouse/Touch/Keyboard event
Canvas->>PointerController: Event handler
PointerController->>Camera: Update pan/zoom state
PointerController->>PointerController: Adjust pointer coords for camera
PointerController->>ZigLogic: Forward adjusted pointer event
Camera->>Canvas: (On zoom/pan) Triggers render scale update
Canvas->>ZigLogic: Call update_render_scale(render_scale)
ZigLogic->>Canvas: Render with updated scale and camera transform
Estimated code review effort4 (~90 minutes) Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/index.ts (1)
193-193: Inconsistent coordinate system usage in asset loading.This line still uses
canvas.clientWidthandcanvas.clientHeightinstead ofprojectWidthandprojectHeight, which creates inconsistency with the new coordinate system used elsewhere.Apply this fix for consistency:
- points: getDefaultPoints(width, height, canvas.clientWidth, canvas.clientHeight), + points: getDefaultPoints(width, height, projectWidth, projectHeight),
🧹 Nitpick comments (2)
src/WebGPU/pointer.ts (2)
159-159: Remove outdated comment.This comment appears to reference old code and should be removed.
- // pointer.zoom = clamp(pointer.zoom + event.deltaY * 0.01, 0.1, 100)
180-198: Consider using pinch gesture for more natural touch zoom.The current implementation uses vertical movement of one finger. A pinch gesture using the distance between two fingers would be more intuitive.
Here's an improved implementation:
- let lastTouchY: number + let lastTouchDistance: number canvas.addEventListener('touchstart', (event) => { if (event.touches.length === 2) { event.preventDefault() - lastTouchY = event.touches[0].clientY + const dx = event.touches[0].clientX - event.touches[1].clientX + const dy = event.touches[0].clientY - event.touches[1].clientY + lastTouchDistance = Math.sqrt(dx * dx + dy * dy) } }) canvas.addEventListener('touchmove', (event) => { if (event.touches.length === 2) { event.preventDefault() - const delta = lastTouchY - event.touches[0].clientY - lastTouchY = event.touches[0].clientY + const dx = event.touches[0].clientX - event.touches[1].clientX + const dy = event.touches[0].clientY - event.touches[1].clientY + const distance = Math.sqrt(dx * dx + dy * dy) + const delta = (lastTouchDistance - distance) * 0.01 + lastTouchDistance = distance camera.zoom = clamp(camera.zoom - delta * 0.01, 0.1, 20) onZoom() } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
integration-tests/template.html(1 hunks)src/WebGPU/pick.ts(1 hunks)src/WebGPU/pointer.ts(5 hunks)src/getCanvasMatrix.ts(1 hunks)src/index.ts(4 hunks)src/logic/index.d.ts(1 hunks)src/logic/index.zig(9 hunks)src/logic/transform_ui.zig(5 hunks)
🧬 Code Graph Analysis (4)
src/getCanvasMatrix.ts (2)
src/utils/mat4.ts (1)
ortho(36-58)src/WebGPU/pointer.ts (1)
camera(20-24)
src/WebGPU/pick.ts (1)
src/WebGPU/pointer.ts (1)
pointer(26-35)
src/index.ts (4)
src/logic/index.d.ts (2)
init_state(21-21)update_render_scale(31-31)src/WebGPU/pointer.ts (2)
camera(20-24)initMouseController(37-199)src/WebGPU/canvasSizeObserver.ts (1)
canvasSizeObserver(11-42)src/utils/getDefaultPoints.ts (1)
getDefaultPoints(3-21)
src/WebGPU/pointer.ts (2)
src/logic/index.d.ts (2)
on_pointer_move(29-29)on_pointer_down(27-27)src/utils/clamp.ts (1)
clamp(1-3)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/getCanvasMatrix.ts (2)
src/utils/mat4.ts (1)
ortho(36-58)src/WebGPU/pointer.ts (1)
camera(20-24)
src/WebGPU/pick.ts (1)
src/WebGPU/pointer.ts (1)
pointer(26-35)
src/index.ts (4)
src/logic/index.d.ts (2)
init_state(21-21)update_render_scale(31-31)src/WebGPU/pointer.ts (2)
camera(20-24)initMouseController(37-199)src/WebGPU/canvasSizeObserver.ts (1)
canvasSizeObserver(11-42)src/utils/getDefaultPoints.ts (1)
getDefaultPoints(3-21)
src/WebGPU/pointer.ts (2)
src/logic/index.d.ts (2)
on_pointer_move(29-29)on_pointer_down(27-27)src/utils/clamp.ts (1)
clamp(1-3)
🔇 Additional comments (14)
integration-tests/template.html (1)
23-23: Good addition to prevent scroll interference with canvas interactions.The
overscroll-behavior: none;property appropriately prevents browser default scroll behaviors from interfering with the new camera panning and zooming functionality.src/logic/index.d.ts (1)
31-31: Correct type declaration for the new render scale function.The function signature appropriately declares the
update_render_scalefunction that takes a numeric render scale parameter.src/WebGPU/pick.ts (1)
88-92: Good alignment with pixel-based coordinate system.Using
canvas.widthandcanvas.heightinstead ofcanvas.clientWidthandcanvas.clientHeightcorrectly aligns the picking matrix calculations with the actual pixel dimensions used in the rendering pipeline, ensuring accurate coordinate mapping with the new camera system.src/getCanvasMatrix.ts (2)
2-2: Appropriate import for camera integration.The camera import correctly brings in the camera state needed for the matrix transformations.
5-16: Well-implemented camera transformation integration.The matrix transformation sequence is correct: first create the orthographic projection, then apply camera translation, and finally apply zoom scaling. The comments provide helpful context about the design decision to handle zoom at the matrix level rather than per-effect.
src/index.ts (6)
15-17: Appropriate imports for camera integration.The new imports correctly bring in the render scale update function and camera object needed for the enhanced functionality.
64-67: Good setup of project dimensions for camera system.Using half the canvas client dimensions for project size creates an appropriate coordinate space for the camera system with panning and zooming capabilities.
80-82: Well-implemented render scale calculation.The
updateRenderScalefunction correctly computes the scale factor based on the ratio of actual canvas pixels to client pixels, adjusted by camera zoom. This ensures consistent rendering across different device pixel ratios and zoom levels.
84-92: Proper camera offset initialization and scale updates.The initial camera offset setup centers the view appropriately, and the render scale update ensures consistent scaling behavior during canvas resizing.
96-96: Correct integration with mouse controller.Passing
updateRenderScaleas a callback to the mouse controller enables dynamic render scale updates during zoom operations.
150-150: Consistent use of project dimensions for new images.Using
projectWidthandprojectHeightfor default points calculation aligns with the new coordinate system.src/logic/transform_ui.zig (3)
150-206: LGTM: Consistent scaling implementation for UI transform lines.The render_scale parameter is properly applied to all geometric calculations while maintaining proportional relationships between corner lines, straight lines, and the rotation icon distance.
211-259: LGTM: Comprehensive scaling implementation for transform UI rendering.All UI elements are consistently scaled including line thicknesses, icon sizes, positioning offsets, and the outer line border. The function signature update and parameter passing to
get_points_of_lineis correct.
262-272: LGTM: Picking functionality properly scaled to match rendering.The scaling implementation maintains consistency between the render and pick functions, using the same thickness values and border calculations. Function signature and parameter passing are correct.
|
🎉 This PR is included in version 0.1.0-next.18 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes