refactor: enhance dependency installation and OpenCL support in Docke…#551
refactor: enhance dependency installation and OpenCL support in Docke…#551felix-schultz merged 1 commit intoalphafrom
Conversation
…rfile and workflows
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Code Review
This pull request adds OpenCL dependencies to the build environment and refactors the website's hero section to improve performance by reducing strand counts and implementing an IntersectionObserver to pause animations. It also introduces Windows-specific logic for screen template matching in the automation package. Review feedback highlights a potential performance bottleneck in the web animation due to layout reflows on scroll, suggests using performance.now() for smoother timing, and identifies issues in the Windows automation implementation regarding redundant screen captures and missing DPI scaling for coordinate conversion.
| pub fn find_template_in_image( | ||
| _screen: &GrayImage, | ||
| template: &GrayImage, | ||
| precision: f32, | ||
| ) -> Vec<(u32, u32, f32)> { | ||
| let mut gui = match rustautogui::RustAutoGui::new(false) { | ||
| Ok(gui) => gui, | ||
| Err(err) => { | ||
| tracing::warn!("Failed to initialize RustAutoGui for template matching: {}", err); | ||
| return Vec::new(); | ||
| } | ||
| }; | ||
|
|
||
| if let Err(err) = gui.prepare_template_from_imagebuffer( | ||
| template.clone(), | ||
| None, | ||
| rustautogui::MatchMode::Segmented, | ||
| ) { | ||
| tracing::warn!("Failed to prepare template image: {}", err); | ||
| return Vec::new(); | ||
| } | ||
|
|
||
| gui.find_image_on_screen(precision) | ||
| .ok() | ||
| .flatten() | ||
| .unwrap_or_default() | ||
| } |
There was a problem hiding this comment.
The Windows implementation of find_template_in_image ignores the provided _screen buffer and performs its own screen capture via gui.find_image_on_screen. This leads to redundant captures and potential inconsistencies if the screen content changes between the caller's capture and this function's internal capture. Consider using the same segmented_ncc logic as the non-Windows implementation to ensure consistency and efficiency, which would require enabling the dev feature for rustautogui on Windows as well.
| pub fn physical_to_logical(x: u32, y: u32) -> (i32, i32) { | ||
| (x as i32, y as i32) | ||
| } |
There was a problem hiding this comment.
The Windows implementation of physical_to_logical does not account for DPI scaling. Since xcap (used in capture_screen_grayscale) returns physical pixels, and mouse coordinates on Windows are typically logical, this will result in incorrect positioning on high-DPI displays. You should use the same scaling logic as the non-Windows implementation.
| pub fn physical_to_logical(x: u32, y: u32) -> (i32, i32) { | |
| (x as i32, y as i32) | |
| } | |
| pub fn physical_to_logical(x: u32, y: u32) -> (i32, i32) { | |
| let (phys_w, phys_h) = xcap::Monitor::all() | |
| .ok() | |
| .and_then(|m| m.into_iter().next()) | |
| .and_then(|m| Some((m.width().ok()?, m.height().ok()?))) | |
| .unwrap_or((1, 1)); | |
| let gui = rustautogui::RustAutoGui::new(false).ok(); | |
| let (logical_w, logical_h) = gui | |
| .map(|mut g| g.get_screen_size()) | |
| .unwrap_or((phys_w as i32, phys_h as i32)); | |
| let scale_x = logical_w as f64 / phys_w as f64; | |
| let scale_y = logical_h as f64 / phys_h as f64; | |
| ((x as f64 * scale_x) as i32, (y as f64 * scale_y) as i32) | |
| } |
|
|
||
| updateSvgBounds(); | ||
| window.addEventListener("resize", updateSvgBounds, { passive: true }); | ||
| window.addEventListener("scroll", updateSvgBounds, { passive: true }); |
There was a problem hiding this comment.
Updating SVG bounds on every scroll event is a performance bottleneck because getBoundingClientRect() triggers a synchronous layout reflow. Since this is used for an animation loop, consider throttling this update or using a ResizeObserver combined with a more efficient way to track scroll position (e.g., calculating the offset once and adjusting based on window.scrollY).
| frameId = null; | ||
| if (!isPageVisible || !isSectionVisible) return; | ||
|
|
||
| const time = Date.now() * 0.001; |
There was a problem hiding this comment.
Using Date.now() for animation timing is less precise and can be affected by system clock changes. performance.now() is preferred for smoother, monotonic timing in animations.
const time = performance.now() * 0.001;
…rfile and workflows