Develop #6
Open
Develop #6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Replace separate sphere/plane/cylinder intersection functions with unified intersect_object_new(). Add intersect_cyl_new.c for cylinder body/cap intersection. Remove legacy intersect_cylinder.c and intersections.c. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Add scene_create/scene_destroy with flag-based element tracking - Extract t_mlx_context from scattered void pointers - Add error.h with typed error codes and error_print/error_exit - Add overlay.h for HUD/keyguide rendering interface - Remove timer_basic.c (consolidated into timer.c) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Replace individual dirty/is_rendering/low_quality booleans with bit-flag state_flags in t_render - Add render_has_flag/render_set_flag/render_clear_flag helpers - Simplify object selection into single window_selection.c - Remove window_select_cycle.c and window_select_helpers.c - Update spatial/render modules for unified object refs Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Adapt HUD rendering to t_object union and t_object_list - Update parse_elements/parse_objects for scene flag helpers - Add parse_cylinder.c for cylinder-specific parsing - Update shadow_test and keyguide for new field paths Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Reject files without .rt extension before attempting to parse. Outputs 'Error\n' followed by descriptive message per eval spec. Implements: FR-001, FR-002 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- J/K: increase/decrease sphere diameter or cylinder diameter - N/M: increase/decrease cylinder height - Minimum threshold 0.1 prevents zero-size objects - Plane resize ignored silently (FR-011) - Sets RENDER_BVH_DIRTY flag for lazy BVH rebuild Implements: FR-003, FR-004, FR-010, FR-011 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- U/O: X-axis rotation, Y/P: Y-axis, LEFT/RIGHT: Z-axis - Rodrigues rotation formula with 5-degree step - Axis degeneration guard: skip rotation if normalized length < EPSILON - Sphere rotation ignored (FR-006) - Sets RENDER_BVH_DIRTY flag for lazy BVH rebuild Implements: FR-005, FR-006, FR-012 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Add KEY_J/K/N/M/U/O/Y/P/LEFT/RIGHT constants (macOS + Linux) - Dispatch resize/rotation keys from handle_transform_keys() - Register expose hook (event 12) for window restore content persistence - Add BVH dirty rebuild before render pass in render_loop() - Set RENDER_BVH_DIRTY on object move for accurate BVH after translation Implements: FR-007, FR-009 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Add scene/, mlx_context, render_flags_set, intersect_object, intersect_cyl_new, parse_cylinder, window_resize, window_rotate - Remove intersect_cylinder, intersections, timer_basic, window_select_cycle, window_select_helpers Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- resize_test.rt: sphere + cylinder for resize key verification - rotation_test.rt: cylinder + plane for rotation key verification - inside_test.rt: camera inside cylinder for interior rendering test Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- spec.md: 13 functional requirements, 6 success criteria - plan.md: 8-phase implementation plan with 6 design decisions - research.md: key mapping, BVH rebuild, expose, normal flip, threshold - data-model.md: RENDER_BVH_DIRTY flag, key constants, state flows - quickstart.md: test scenes and verification checklists - tasks.md: 27 tasks across 9 phases (all completed) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Add missing features for mandatory evaluation: - .rt file extension validation - Object resize (J/K/N/M keys) - Object rotation (U/O/Y/P/LEFT/RIGHT keys) - Window expose handler for content persistence - Cylinder inside normal correction - BVH lazy rebuild on object transform - Unified object representation refactoring
…facts Add feature spec, plan, research, data-model, quickstart, tasks, and requirements checklist for HUD key guide update and expose restore fix. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Add keyguide_render_extra.c with Resize (J/K - Diameter, N/M - Height) and Rotation (U/O - Rot X, Y/P - Rot Y, <-/-> - Rot Z) sections. Increase KEYGUIDE_HEIGHT from 400 to 500 to accommodate new labels. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Remove hud_render_background() call to eliminate pixel-by-pixel blending overhead. Set RENDER_DIRTY on HUD toggle to ensure clean scene buffer after overlay text is removed. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Call hud_render() and keyguide_render() in handle_expose() when HUD is visible, so overlays persist after window uncover/restore. Remove unused is_movement_key static function. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…store - Add Resize/Rotation key sections to keyguide - Restore HUD/keyguide overlay on expose event - Remove HUD/keyguide background blending - Remove unused is_movement_key function
…m compliance Extract debounce_handle_active() and debounce_handle_preview() from debounce_update() to stay within 25-line limit. Move debounce_cancel() to render_debounce_timer.c to stay within 5-function limit. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Remove unused metrics_log_render(). Move metrics_reset_bvh() from metrics_counters.c to metrics_calc.c to stay within 5-function limit. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Fix extra tab indentation in bounds_for_plane(). Move get_object_center() to bvh_init.c to stay within 5-function limit. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Remove forward declaration from intersect_object.c to fix Norm tab indentation error. Add prototype to ray.h instead. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Rename anonymous union to union u_object_data to satisfy Norm v4.1 union naming convention (u_ prefix required). Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Move g_error_messages from file-scope global to function-local static const in error_get_message(). Rename to error_messages to follow local variable naming convention. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- Add libft as git submodule in lib/libft - Update Makefile to build and link libft - Add libft include path to CFLAGS - Remove minilibx-linux submodule (use system install) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Delete ft_atoi.c (now provided by libft) - Delete memory.c (ft_memcpy, ft_memset now from libft) - Add format_helpers.c with format_id and float_to_str - Update utils.h with new format helper declarations - Include libft.h in minirt.h, remove ft_atoi declaration Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use format_id instead of snprintf for sphere/plane/cylinder IDs - Replace strcpy with ft_strlcpy in read_line - Remove stdio.h/string.h includes where possible Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use ft_memcpy instead of memcpy in object_list_grow - Use ft_bzero instead of memset in scene_create - Remove string.h includes Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use ft_bzero for struct initialization - Include libft.h instead of string.h Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add <stdlib.h> header to render_quality.c to ensure proper standard library function availability and fix comment line wrapping.
…aders relocate parser helper and error prototypes into includes/parser.h drop redundant forward declarations from source files reorder static helpers to avoid local prototypes in pixel_timing_print.c
Uninitialized temp_hit.distance caused intersect_ref() to use a garbage upper bound, producing non-deterministic hit results when BVH is active. Initialize to hit->distance to preserve the caller's clip distance. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Ray direction components near zero caused NaN from 0/0 division, breaking all comparisons. Add safe_slab_axis() helper using t_axis_check struct to handle parallel rays via origin-in-slab test. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Cross product with world up (0,1,0) yields zero vector when camera direction is near-vertical, producing NaN in the basis. Switch to alternative up vector (0,0,1) when dot product exceeds 0.999. Co-Authored-By: Claude Opus 4.5 <[email protected]>
tiles_x was computed from hardcoded 800 instead of real window width, and tile clamping used WINDOW_WIDTH/HEIGHT macros. Store width/height in t_progressive_state and reference them in progressive_next_tile(). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Shadow bias was computed with hardcoded (0,1,0) normal regardless of surface orientation, causing acne on vertical walls and peter-panning on slopes. Introduce t_shadow_query to bundle point+normal within the 4-argument Norm limit and pass hit->normal from the caller. Co-Authored-By: Claude Opus 4.5 <[email protected]>
read() returning -1 was treated as EOF, silently discarding I/O errors mid-parse. Add io_error flag to t_line_reader, set it in refill_buffer on read failure, check it after the parsing loop, and report via the new PARSE_ERR_IO error code with a descriptive message. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Signed integer overflow from result * 10 + digit is undefined behavior. Add pre-multiplication check against INT_MAX and return failure on overflow, propagated as PARSE_ERR_NUMBER_FORMAT to the caller. Co-Authored-By: Claude Opus 4.5 <[email protected]>
capacity * 2 could overflow INT_MAX for extremely large lists. Check capacity > INT_MAX / 2 before doubling and fail gracefully. Co-Authored-By: Claude Opus 4.5 <[email protected]>
mlx_img_put_pixel/get_pixel had no bounds validation, risking buffer overrun on out-of-range coordinates. Add width/height fields to t_mlx_img, store them in mlx_img_init, and check bounds before computing byte offsets. Co-Authored-By: Claude Opus 4.5 <[email protected]>
close_window called pixel_timing_cleanup, keyguide_cleanup, and hud_cleanup before cleanup_all, which invokes render_destroy that performs the same cleanup. Remove the redundant calls to maintain a single cleanup path via render_destroy. Co-Authored-By: Claude Opus 4.5 <[email protected]>
init_ui_components freed only the render struct on hud_init or keyguide_init failure, leaking the MLX window and image. Delegate cleanup responsibility to render_create which now calls render_destroy on failure, properly releasing all initialized resources. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Include spec, plan, research, data-model, quickstart, checklist, contracts, and tasks for the code hardening feature covering 9 functional requirements identified from code review. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Code hardening: fix 9 defects from code review + 2 review findings. High priority (rendering correctness): - FR-001: BVH temp_hit.distance initialization - FR-002: AABB slab test div-by-zero protection Medium priority (specific conditions): - FR-003: Camera gimbal lock prevention - FR-004: Progressive tile hardcoded width removal - FR-005: Shadow bias actual surface normal - FR-006: I/O error detection in line reader Low priority (defensive coding): - FR-007: Integer overflow detection in parser - FR-008: Object list capacity overflow guard - FR-009: Pixel coordinate bounds checking Review findings: - Remove duplicate cleanup calls in close_window - Fix MLX resource leak on UI initialization failure
…ifacts Add feature specification, implementation plan, research findings, data model, deletion manifest, quickstart guide, and task list for dead code removal based on full codebase review. Reference: docs/codebase-review-2026-02-04.md Co-Authored-By: Claude Opus 4.5 <[email protected]>
Delete render_state, render_quality, render_progressive files entirely. These modules defined t_render_state and related types/functions that were never integrated into t_render (window.h), leaving the entire module tree unreachable. Deleted files (3 headers + 3 sources): - includes/render_state.h (t_render_state, t_quality_mode, etc.) - includes/render_quality.h - includes/render_progressive.h - src/render/render_state.c (render_state_init, render_state_update) - src/render/render_quality.c (quality_set_mode, quality_should_upgrade, etc.) - src/render/render_progressive.c (progressive_init, progressive_next_tile, etc.) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Delete files with zero callers and no header declarations. Deleted files (2 headers + 2 sources): - includes/overlay.h (4 unused types, 8 unimplemented function decls) - includes/format_object_id.h (unused header) - src/spatial/aabb_shapes.c (aabb_for_sphere, aabb_for_cylinder, aabb_for_plane) - src/utils/format_object_id.c (format_object_id, get_object_type_prefix) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Clean partial dead code from headers and one source file. - objects.h: delete legacy t_sphere, t_plane, t_cylinder typedefs (superseded by t_object + t_sphere_data/t_plane_data/t_cylinder_data) - parser.h: delete dead parse_vector, parse_color declarations (superseded by parse_vector_strict, parse_color_strict) - window_internal.h: delete 7 unimplemented selection helper declarations (cycle_type_forward, cycle_backward_*, next_type_from_*) and 1 duplicate render_scene_to_buffer declaration - keyguide.h: delete keyguide_render_background declaration - keyguide_render.c: delete keyguide_render_background function body Co-Authored-By: Claude Opus 4.5 <[email protected]>
Move in_range() from parse_validation.c to parse_validation_strict.c,
then delete emptied legacy files and migrate error API.
- Move in_range() to parse_validation_strict.c (parser.h already declares it)
- Delete parse_validation.c (parse_vector, parse_color were dead code)
- Delete ft_atof.c (only caller was dead parse_vector)
- Replace 7 print_error() calls in parser.c with error_print() using
comma operator pattern (error_print(CODE), 0) to preserve return value
Mapping: validate_scene 4x → ERR_PARSE_MISSING,
parse_scene → ERR_FILE_EXT, ERR_FILE_OPEN, ERR_MALLOC
- Delete print_error() function from error.c
- Delete print_error declaration from error.h
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace cleanup_all() in close_window() with explicit render_destroy + scene_destroy sequence. Save render->scene to local variable before render_destroy to prevent use-after-free. - window_lifecycle.c: cleanup_all → render_destroy + scene_destroy - Delete cleanup.c (cleanup_scene, cleanup_render, cleanup_all wrappers) - minirt.h: remove cleanup_all and ft_atof declarations - Makefile: remove 8 SRCS entries for all deleted .c files Co-Authored-By: Claude Opus 4.5 <[email protected]>
Dead code removal: 13 files deleted, 22 declarations removed, 3 legacy wrapper call sites replaced with direct calls. Zero functional change — build, norminette, error output preserved.
mlx_context_init() leaked mlx connection and window handle when mlx_new_window or mlx_img_init failed. Add mlx_context_destroy() calls in failure paths to clean up partially initialized resources. Also add defensive mlx_context_destroy in render_create failure path. Co-Authored-By: Claude Opus 4.5 <[email protected]>
HUD and keyguide allocated bg_img via mlx_new_image but never rendered them. HUD background uses direct alpha blending on the main framebuffer (hud_text.c), keyguide renders text only via mlx_string_put. - Remove bg_img, bg_data, bpp, size_line, endian fields from t_hud_state and t_keyguide_state (window.h) - Delete fill_background_pixels() and hud_create_background() from hud_init.c (5→3 functions) - Simplify keyguide_init() to position/visibility init only - Simplify hud_cleanup() and keyguide_cleanup() to no-ops (API signatures preserved for render_destroy compatibility) Saves ~3.3MB unnecessary image allocation per session. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Delete hud_create_background declaration from hud.h (no implementation) - Remove redundant mlx_context_destroy in render_create failure path (mlx_context_init already cleans up internally on failure) - Update hud_init/hud_cleanup/init_ui_components comments to reflect bg_img removal from prior refactor Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Related Issue
Closes #
Type of Change
Changes
Testing
Test Environment
Test Steps
Test Results
Screenshots
Checklist
Constitution Compliance
Performance Impact
Additional Notes