Native Screenshot Feel by AryanRogye · Pull Request #12 · AryanRogye/ComfyMark

AryanRogye · 2025-09-12T20:09:52Z

No description provided.

… i get the image

Copilot

Pull Request Overview

This PR enhances the screenshot functionality with screen-under-mouse detection and cropping support, creating a more "native screenshot feel" for multi-screen setups.

Key changes:

Modified screenshot provider protocol to support cropping rectangles
Moved cropping logic from AppCoordinator to ScreenshotService
Added comprehensive test coverage for screenshot functionality

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ComfyMarkTests/Screenshot/ScreenshotProviderTests.swift	Comprehensive test suite covering basic functionality, edge cases, and concurrency scenarios
ComfyMarkTests/Screenshot/MockScreenshotProvider.swift	Mock implementation with configurable behavior for testing
ComfyMarkTests/ComfyMarkTests.swift	Removed old test file to consolidate testing structure
ComfyMark/Services/ScreenshotService.swift	Added cropping parameter to protocol and moved pixel crop calculation logic
ComfyMark/Coordinators/SelectionOverlayCoordinator.swift	Added screen detection and overlay recreation for different screens
ComfyMark/App/AppCoordinator.swift	Refactored to use new cropping API and extracted coordinator configuration
ComfyMark.xcodeproj/project.pbxproj	Updated project structure to include new test files

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-12T20:29:51Z

+        let image = await mockProvider.takeScreenshot()
+
+        XCTAssertNotNil(image)
+
+        // Test that we can create NSImage from CGImage
+        let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))
+        XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")
+
+        // Test image data exists and has expected size
+        let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)
+        if let dataProvider = image!.dataProvider,


Force unwrapping is used multiple times on the same optional image. Consider using a guard statement or if-let binding to safely unwrap once and improve readability.

Suggested change

let image = await mockProvider.takeScreenshot()

XCTAssertNotNil(image)

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image!.dataProvider,

let imageOpt = await mockProvider.takeScreenshot()

guard let image = imageOpt else {

XCTFail("Image should not be nil")

return

}

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image, size: NSSize(width: image.width, height: image.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image.width * image.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image.dataProvider,

Copilot · 2025-09-12T20:29:52Z

+        let image = await mockProvider.takeScreenshot()
+
+        XCTAssertNotNil(image)
+
+        // Test that we can create NSImage from CGImage
+        let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))
+        XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")
+
+        // Test image data exists and has expected size
+        let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)
+        if let dataProvider = image!.dataProvider,


Force unwrapping is used multiple times on the same optional image. Consider using a guard statement or if-let binding to safely unwrap once and improve readability.

Suggested change

let image = await mockProvider.takeScreenshot()

XCTAssertNotNil(image)

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image!.dataProvider,

let imageOpt = await mockProvider.takeScreenshot()

guard let image = imageOpt else {

XCTFail("Image should not be nil")

return

}

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image, size: NSSize(width: image.width, height: image.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image.width * image.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image.dataProvider,

Copilot · 2025-09-12T20:29:52Z

+        XCTAssertNotNil(image)
+
+        // Test that we can create NSImage from CGImage
+        let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))
+        XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")
+
+        // Test image data exists and has expected size
+        let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)
+        if let dataProvider = image!.dataProvider,


Force unwrapping is used multiple times on the same optional image. Consider using a guard statement or if-let binding to safely unwrap once and improve readability.

Suggested change

XCTAssertNotNil(image)

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image!.dataProvider,

guard let image = image else {

XCTFail("Image should not be nil")

return

}

// Test that we can create NSImage from CGImage

let nsImage = NSImage(cgImage: image, size: NSSize(width: image.width, height: image.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image.width * image.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image.dataProvider,

Copilot · 2025-09-12T20:29:52Z

+        let width = max(0, Int(rect.width))
+        let height = max(0, Int(rect.height))
+
+        // Return nil for zero dimensions if that's your expected behavior
+        guard width > 0 && height > 0 else { return nil }
+
+        return makeTestImage(w: width, h: height)
+    }
+
+    private func makeTestImage(w: Int, h: Int) -> CGImage? {
+        // Handle zero or negative dimensions
+        guard w > 0 && h > 0 else { return nil }
+
+        let colorSpace = CGColorSpaceCreateDeviceRGB()
+        guard let ctx = CGContext(
+            data: nil,
+            width: w,
+            height: h,
+            bitsPerComponent: 8,
+            bytesPerRow: w * 4,
+            space: colorSpace,
+            bitmapInfo: CGImageAlphaInfo.premultipliedLast.rawValue
+        ) else { return nil }
+
+        // Create a more interesting test pattern instead of solid color
+        ctx.setFillColor(NSColor.systemBlue.cgColor)
+        ctx.fill(CGRect(x: 0, y: 0, width: w, height: h))
+
+        // Add some pattern to make images more distinguishable
+        ctx.setFillColor(NSColor.white.cgColor)
+        ctx.fill(CGRect(x: 0, y: 0, width: min(w, 10), height: min(h, 10)))


The test case testZeroDimensionsCrop expects zero dimensions to potentially return an image with width/height of 0 or 1, but this guard statement always returns nil for zero dimensions, making the test assertions inconsistent with the implementation.

Suggested change

let width = max(0, Int(rect.width))

let height = max(0, Int(rect.height))

// Return nil for zero dimensions if that's your expected behavior

guard width > 0 && height > 0 else { return nil }

return makeTestImage(w: width, h: height)

}

private func makeTestImage(w: Int, h: Int) -> CGImage? {

// Handle zero or negative dimensions

guard w > 0 && h > 0 else { return nil }

let colorSpace = CGColorSpaceCreateDeviceRGB()

guard let ctx = CGContext(

data: nil,

width: w,

height: h,

bitsPerComponent: 8,

bytesPerRow: w * 4,

space: colorSpace,

bitmapInfo: CGImageAlphaInfo.premultipliedLast.rawValue

) else { return nil }

// Create a more interesting test pattern instead of solid color

ctx.setFillColor(NSColor.systemBlue.cgColor)

ctx.fill(CGRect(x: 0, y: 0, width: w, height: h))

// Add some pattern to make images more distinguishable

ctx.setFillColor(NSColor.white.cgColor)

ctx.fill(CGRect(x: 0, y: 0, width: min(w, 10), height: min(h, 10)))

let width = Int(rect.width)

let height = Int(rect.height)

// Allow zero or one dimensions for test purposes

let safeWidth = max(0, width)

let safeHeight = max(0, height)

return makeTestImage(w: safeWidth, h: safeHeight)

}

private func makeTestImage(w: Int, h: Int) -> CGImage? {

// Allow zero or one dimensions for test purposes

let width = max(0, w)

let height = max(0, h)

// If both dimensions are zero, return nil (CGContext cannot create 0x0 images)

if width == 0 && height == 0 { return nil }

let colorSpace = CGColorSpaceCreateDeviceRGB()

guard let ctx = CGContext(

data: nil,

width: width,

height: height,

bitsPerComponent: 8,

bytesPerRow: max(1, width) * 4,

space: colorSpace,

bitmapInfo: CGImageAlphaInfo.premultipliedLast.rawValue

) else { return nil }

// Create a more interesting test pattern instead of solid color

ctx.setFillColor(NSColor.systemBlue.cgColor)

ctx.fill(CGRect(x: 0, y: 0, width: width, height: height))

// Add some pattern to make images more distinguishable

ctx.setFillColor(NSColor.white.cgColor)

ctx.fill(CGRect(x: 0, y: 0, width: min(width, 10), height: min(height, 10)))

Copilot · 2025-09-12T20:29:52Z

-            return try await captureScreen(screen, content: content, excludedApps: excludedApps)
+            let image = try await captureScreen(screen, content: content, excludedApps: excludedApps)
+
+            /// Get Clamped Pixel Rect


The comment uses incorrect grammar. Should be 'Get clamped pixel rect' (lowercase 'clamped').

Suggested change

/// Get Clamped Pixel Rect

/// Get clamped pixel rect

Copilot

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-13T03:22:32Z

+        let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))
+        XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")
+
+        // Test image data exists and has expected size
+        let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)
+        if let dataProvider = image!.dataProvider,
+           let data = dataProvider.data {
+            let actualDataSize = CFDataGetLength(data)
+            XCTAssertEqual(actualDataSize, expectedDataSize, "Image data size should match expected size")
+        } else {
+            XCTFail("Image should have valid data provider and data")
+        }
+    }


Avoid using force unwrapping with !. The image has already been verified as non-nil in the guard statement above, so this should use safe unwrapping.

Suggested change

let nsImage = NSImage(cgImage: image!, size: NSSize(width: image!.width, height: image!.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image!.width * image!.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image!.dataProvider,

let data = dataProvider.data {

let actualDataSize = CFDataGetLength(data)

XCTAssertEqual(actualDataSize, expectedDataSize, "Image data size should match expected size")

} else {

XCTFail("Image should have valid data provider and data")

}

}

if let image = image {

let nsImage = NSImage(cgImage: image, size: NSSize(width: image.width, height: image.height))

XCTAssertNotNil(nsImage, "Should be able to create NSImage from CGImage")

// Test image data exists and has expected size

let expectedDataSize = image.width * image.height * 4 // 4 bytes per pixel (RGBA)

if let dataProvider = image.dataProvider,

let data = dataProvider.data {

let actualDataSize = CFDataGetLength(data)

XCTAssertEqual(actualDataSize, expectedDataSize, "Image data size should match expected size")

} else {

XCTFail("Image should have valid data provider and data")

}

}

… feeling be fast

Easily Allow Open Settings with Command , Easily Change Screenshot Side from left to right Made default Viewport Scale out just 0.9

…, though this is the default option for now

AryanRogye added 2 commits September 12, 2025 15:09

cleaning up the screenshot service before I work on what happens once…

fbc3177

… i get the image

organized app coordinator, now i'm ready to start

9caae41

AryanRogye requested a review from Copilot September 12, 2025 20:25

Copilot AI reviewed Sep 12, 2025

View reviewed changes

added the ability to show the image in the bottom

29c751d

AryanRogye requested a review from Copilot September 13, 2025 03:21

Copilot AI reviewed Sep 13, 2025

View reviewed changes

AryanRogye added 10 commits September 13, 2025 09:56

making things less confusing for users, i'm really trying to make the…

ba5c26f

… feeling be fast

Easily Allow Open Settings with Command ,

c29e13c

Easily Allow Open Settings with Command , Easily Change Screenshot Side from left to right Made default Viewport Scale out just 0.9

added new 'fake genie' window effect

7771818

added new ability for the native screenshot behavior to be chosen off…

d9e7591

…, though this is the default option for now

making things much more safer, adding new option for dismiss timer

116df72

added new timer features

ed7a17d

sync

39828ec

transfered the screenshot to the SnapCore

dac769f

added stroke kind

3307771

added undo/redo

461175d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native Screenshot Feel#12

Native Screenshot Feel#12
AryanRogye wants to merge 13 commits intomainfrom
feature/native_screenshot_feel

AryanRogye commented Sep 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 12, 2025

Uh oh!

Copilot AI Sep 12, 2025

Uh oh!

Copilot AI Sep 12, 2025

Uh oh!

Copilot AI Sep 12, 2025

Uh oh!

Copilot AI Sep 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

AryanRogye commented Sep 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 13, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants