Fix/dock orientation #3

MrJarnould · 2026-01-13T15:13:34Z

Summary

This PR addresses several issues preventing reliable Dock screenshots on macOS, specifically on Retina displays and systems with non-standard Dock configurations.

1. Hardcoded Dock Configuration

Problem: The previous implementation assumed the Dock was always on the bottom and set to autohide=True. This caused failures for users with the Dock on the left/right or always visible.

Solution:

Added get_dock_orientation(): Executes defaults read com.apple.dock orientation to detect if the Dock is left, right, or bottom.
Added get_dock_autohide(): Executes defaults read com.apple.dock autohide.
The DockCapture class now defaults to these system settings instead of hardcoded values.

2. Unreliable Dock Reveal

Problem: The mouse movement logic used a small offset (e.g., screen_width // 2, screen_height - 2), which often failed to trigger the Dock reveal animation reliably.

Solution:

Updated cursor logic to target the exact screen edge (e.g., pixel 0 or screen_height - 1).
Added a try/finally block to ensure the mouse cursor is restored to its original position even if capture fails.

3. Low-Resolution & Misaligned Screenshots

Problem:

capture_full_screen used a bbox argument which inadvertently caused ImageGrab to return a downscaled 1x image on Retina displays (logical points instead of physical pixels).
Hardcoded 2x scaling logic in crop_screenshot caused "double-cropping" (capturing empty space) when applied to these 1x images.

Solution:

Capture: Removed the bbox argument to enable native Retina (2x) resolution (e.g., 3024x1964).
Scaling: Refactored crop_screenshot and segment_image to dynamically calculate the scale factor (by comparing image pixels to screen points). This ensures correct cropping and bounding box alignment regardless of whether the source image is 1x or 2x.

mshamrai

Thank you for the PR! We really appreciate the collaboration.
Can you please elaborate on the change in scaling factor calculation?

mshamrai · 2026-01-14T11:48:22Z

macapptree/screenshot_app_window.py



 def crop_screenshot(image_path, window_coords, output_path):
-    backing_scale_factor = AppKit.NSScreen.mainScreen().backingScaleFactor()


Why don't you like this function? It should return the scaling factor, there is no need to compute it manually

I avoided backingScaleFactor because it introduces a critical bug in multi-monitor setups. NSScreen.mainScreen() returns the properties of the primary display (e.g., the MacBook), regardless of where the window is actually located.

I verified this with a physical test case using a Retina MacBook (Main, 2.0x) and an LG External Monitor (Extended, 1.0x).

I moved the Calculator app to the external monitor (1.0x).

mainScreen().backingScaleFactor() still returned 2.0 because the Mac is the primary display.

The script applied 2x math to the 1.0x image, resulting in a broken/blank screenshot because the crop area was doubled and shifted out of bounds.

By calculating the scale manually, we ensure the math matches the actual monitor the window is on.

mshamrai · 2026-01-14T11:56:37Z

macapptree/window_tools.py

+        if scale_factor is None:
+            # Heuristic: Compare image width vs window element width (points) to detect 1x vs 2x.
+            # If explicit detection fails, fallback to the screen's backing scale factor.
+            current_scaling_factor = _screen_scaling_factor


So why don't you like the backing scale factor? Did you have problems with it?

Yes, the problem is that backingScaleFactor is environment-dependent, whereas measuring the image is data-dependent (and therefore reliable).

Since macOS creates screenshots based on the target display's resolution, the image file itself is the source of truth. My implementation divides the Image Width (px) by the Window Width (pts) to derive the exact multiplier used for that specific capture.

Window on Retina: 460px / 230pts = 2.0

Window on External: 230px / 230pts = 1.0

This makes the logic display-agnostic. It works correctly even in mixed-DPI setups, Sidecar, or potentially headless CI environments where mainScreen might report default values that don't match the virtual framebuffer.

mshamrai

I am not sure, that your implementation will return needed value in the case you mention. Unless you find the screen where the window is.

And can you please create a function for the scale_factor calculation in order to reduce code duplicates? You may put it in window_tools.py file.

Much appreciated!

mshamrai · 2026-01-15T12:43:47Z

macapptree/screenshot_app_window.py

-
+
+    # Get main screen width (Points) to determine ratio
+    screen = AppKit.NSScreen.mainScreen()


But here you also use AppKit.NSScreen.mainScreen(), which returns the main screen. It looks like in this case your implementation would return the same value of the scale_factor

Thanks for the feedback. You're right. Comparing the image width to mainScreen in crop_screenshot was incorrect because it re-introduced the same fragility as backingScaleFactor (comparing a window object to a screen reference).

I pushed a refactor that addresses both points:

Shared Helper: moved the scale calculation logic to a reusable function get_image_scale_factor in window_tools.py to remove the code duplication

Logic Fix: In screenshot_app_window.py, updated the logic to pass the window's logical width as the reference target instead of the screen width

# New logic scale_factor = get_image_scale_factor(img_width, window_width)

Ensures the ratio is calculated strictly as Window Pixels / Window Points which guarantees the correct scale (1.0 vs 2.0) regardless of which monitor the window is on.

mshamrai

Good! I think we almost done here. Only some refactoring remains

mshamrai · 2026-01-16T09:31:23Z

macapptree/screenshot_app_window.py

-    frame = screen.frame()
-    screen_width_points = frame.size.width
+    # Local import to avoid circular dependency
+    from macapptree.window_tools import get_image_scale_factor


I don't think the local import is a good practice. Can you please refactor it to avoid local imports and circular dependencies? Maybe move it to a new file?

Done. Created scale_utils.py as a standalone module with no dependencies on window_tools or screenshot_app_window. Both files now import from it at the top level.

mshamrai · 2026-01-16T09:31:45Z

macapptree/screenshot_app_window.py

+        # But get_image_scale_factor handles logic.
+        # Let's check get_image_scale_factor implementation again.
+        # I changed it to return None instead of global _screen_scaling_factor because _screen_scaling_factor is not available here.
+        # So I need to import AppKit here to get fallback.


These comments look totally unnecessary

mshamrai · 2026-01-16T09:32:47Z

macapptree/screenshot_app_window.py

+        # Let's check get_image_scale_factor implementation again.
+        # I changed it to return None instead of global _screen_scaling_factor because _screen_scaling_factor is not available here.
+        # So I need to import AppKit here to get fallback.
+        scale_factor = AppKit.NSScreen.mainScreen().backingScaleFactor()


And this logic is already incapsulated in the function, so I think you should not have it here

removed. The fallback is now handled inside get_image_scale_factor() so callers don't need to check for None.

mshamrai · 2026-01-16T09:34:20Z

macapptree/window_tools.py

+    """
+    if target_width_points > 0:
+        ratio = image_width_px / target_width_points
+        if abs(ratio - 1.0) < 0.1:


Can you please introduce new eps input variable with the default 0.1 instead of using constant as is?

done. Added eps: float = 0.1 as a parameter.

mshamrai · 2026-01-16T09:35:35Z

macapptree/window_tools.py

-                        elif abs(ratio - 2.0) < 0.1:
-                            current_scaling_factor = 2.0
+                    target_width = bx2 - bx
            except Exception:


What exceptions are you expecting here? It looks like it is unnecessary

Removed. Replaced with a simple if hasattr() check.

crawlmacos

Looks good enough. Thank you

- Create scale_utils.py with get_image_scale_factor function - Add eps parameter for ratio tolerance (default 0.1) - Move fallback logic into the function - Remove local imports and unnecessary try/except - Remove debug comments - Fix unused variable warning (img_height -> _)

mshamrai reviewed Jan 14, 2026

View reviewed changes

mshamrai requested changes Jan 15, 2026

View reviewed changes

mshamrai requested changes Jan 16, 2026

View reviewed changes

crawlmacos approved these changes Jan 21, 2026

View reviewed changes

mshamrai approved these changes Jan 21, 2026

View reviewed changes

MrJarnould added 7 commits January 21, 2026 14:00

feat: Detect Dock orientation and autohide status automatically

ab013d3

fix: App uses dynamic Dock detection instead of hardcoded defaults

e737751

fix: Use exact screen edge coordinates to reveal Dock

f200659

fix: Dynamically scale bounding boxes based on screenshot resolution

f481299

fix: Enable Retina capture and refine scaling logic

1764b7f

refactor: move scale logic to window_tools and fix mainScreen dependency

00a0cd8

MrJarnould force-pushed the fix/dock-orientation branch from d635d20 to 1b3c966 Compare January 21, 2026 13:00



		def crop_screenshot(image_path, window_coords, output_path):
		backing_scale_factor = AppKit.NSScreen.mainScreen().backingScaleFactor()



		# Get main screen width (Points) to determine ratio
		screen = AppKit.NSScreen.mainScreen()

Fix/dock orientation #3

Are you sure you want to change the base?

Fix/dock orientation #3

Uh oh!

Conversation

MrJarnould commented Jan 13, 2026

Summary

1. Hardcoded Dock Configuration

2. Unreliable Dock Reveal

3. Low-Resolution & Misaligned Screenshots

Uh oh!

mshamrai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshamrai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshamrai left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crawlmacos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mshamrai left a comment •

edited

Loading