Skip to content

Commit aabfe18

Browse files
RobbieTheWagnercoderabbitai[bot]
RobbieTheWagner
and
coderabbitai[bot]
authored
Implement new color picker with better updates (#2429)
* Get rust one working * Remove exclude window logic * Get squares more static size * Fix color sampling * Fix jumpiness * Try to setup linux support * Update linux.rs * Update linux.rs * Update linux.rs * Try grim * Update linux.rs * Fallback to Electron for Wayland * Update TESTING_THE_SAMPLER.md * Update linux.rs * Fix lint * Update forge.config.ts * Update package.json * Try to fix wayland portal * Try to fix errors * Update wayland_portal.rs * Update wayland_portal.rs * Update linux.rs * Update electron.yml * Another one * Try to fix clicking * Update pipewire * Update wayland_portal.rs * Update README.md * Update README.md * Update wayland_portal.rs * Pipewire 0.8 * Update wayland_portal.rs * Update wayland_portal.rs * Update wayland_portal.rs * Update wayland_portal.rs * Trying to build * Update wayland_portal.rs * Get Wayland kind of working * Try to get things working again * Update windows.rs * Try to fix electron tests * Update package.json * Update electron-app/magnifier/magnifier-main-rust.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update magnifier-main-rust.ts * Update magnifier-main-rust.ts * Update linux.rs * Update linux.rs * Cleanup code * Update wayland_portal.rs * Simplify rust/electron fallback * Various fixes * Move files into one dir, add tests * Ignore target for linting * Fix lint * Update linux_sampler_tests.rs * Fix windows * Update Cargo.toml * Try to fix windows * Update magnifier-main-rust.ts --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 69ca606 commit aabfe18

45 files changed

Lines changed: 6952 additions & 55 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/electron.yml‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,17 @@ jobs:
1313
timeout-minutes: 10
1414
steps:
1515
- uses: actions/checkout@v4
16+
- uses: dtolnay/rust-toolchain@stable
1617
- uses: pnpm/action-setup@v4
1718
- uses: actions/setup-node@v4
1819
with:
1920
node-version: 22
2021
cache: 'pnpm'
21-
- run: pnpm install
22-
- name: Get xvfb
23-
run: sudo apt-get install xvfb
22+
- name: Install system dependencies
23+
run: |
24+
sudo apt-get update
25+
sudo apt-get install -y xvfb libx11-dev pkg-config
26+
- run: pnpm install
2427
- name: Magnifier Test
2528
run: pnpm test:magnifier
2629
- name: Electron Test

‎.github/workflows/rust-tests.yml‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
name: Rust Tests
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
paths:
9+
- 'electron-app/magnifier/rust-sampler/**'
10+
- '.github/workflows/rust-tests.yml'
11+
12+
jobs:
13+
test:
14+
name: Test Rust Sampler
15+
runs-on: ${{ matrix.os }}
16+
strategy:
17+
matrix:
18+
os: [ubuntu-latest, macos-latest, windows-latest]
19+
steps:
20+
- uses: actions/checkout@v4
21+
- uses: dtolnay/rust-toolchain@stable
22+
- name: Install Linux dependencies
23+
if: runner.os == 'Linux'
24+
run: sudo apt-get update && sudo apt-get install -y libx11-dev libpipewire-0.3-dev pkg-config
25+
- uses: pnpm/action-setup@v4
26+
- uses: actions/setup-node@v4
27+
with:
28+
node-version: 22
29+
cache: 'pnpm'
30+
- run: pnpm install
31+
32+
# Run tests with platform-specific features
33+
- name: Run Rust tests (Linux)
34+
if: runner.os == 'Linux'
35+
run: cd electron-app/magnifier/rust-sampler && cargo test --features x11,wayland
36+
37+
- name: Run Rust tests (macOS/Windows)
38+
if: runner.os != 'Linux'
39+
run: cd electron-app/magnifier/rust-sampler && cargo test

‎.prettierignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
# compiled output
55
/dist/
6+
/electron-app/dist/
7+
/electron-app/magnifier/rust-sampler/target/
68

79
# misc
810
/coverage/

‎.stylelintignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@
33

44
# compiled output
55
/dist/
6+
7+
# Rust generated documentation
8+
**/target/doc/

‎README.md‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,29 @@ You will need the following things properly installed on your computer.
2424
- [pnpm](https://pnpm.io/)
2525
- [Ember CLI](https://ember-cli.com/)
2626
- [Google Chrome](https://google.com/chrome/)
27+
- [Rust](https://www.rust-lang.org/) (for building the pixel sampler)
2728

2829
### Linux System Requirements
2930

30-
On Linux, the color picker functionality requires additional system libraries. These dependencies are automatically installed when using the `.deb` package, but if building from source, ensure you have:
31+
On Linux, the color picker functionality requires additional system libraries and tools. These dependencies are automatically installed when using the `.deb` package, but if building from source, ensure you have:
3132

3233
```bash
3334
# Debian / Ubuntu
3435
sudo apt-get install libxcb1 libxrandr2 libdbus-1-3
3536

37+
# For Wayland (recommended)
38+
sudo apt-get install grim
39+
40+
# For X11 (fallback)
41+
sudo apt-get install imagemagick xdotool
42+
# OR
43+
sudo apt-get install scrot xdotool
44+
3645
# Alpine
37-
sudo apk add libxcb libxrandr dbus
46+
sudo apk add libxcb libxrandr dbus grim
3847
```
3948

40-
These libraries are required for the screenshot functionality used by the magnifying color picker.
49+
**Wayland Users:** The `grim` tool is required for screenshot-based color picking on Wayland. See [rust-sampler/README.md](rust-sampler/README.md) for more details.
4150

4251
## Installation
4352

‎app/services/data.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export default class DataService extends Service {
5757
);
5858
reject(
5959
new Error(
60-
`Failed to delete corrupt database: ${deleteRequest.error}`
60+
`Failed to delete corrupt database: ${deleteRequest.error?.message ?? 'Unknown error'}`
6161
)
6262
);
6363
};

‎docs/PERFORMANCE.md‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# Performance Characteristics
2+
3+
## Rust Sampler Performance
4+
5+
### Current Performance (macOS)
6+
7+
The Rust sampler achieves:
8+
9+
- **15-20 FPS** continuous sampling
10+
- **50-70ms** per frame (capture + processing)
11+
- **Live color updates** as you move the cursor
12+
13+
This is limited by macOS's `CGDisplayCreateImageForRect` API, which is the bottleneck.
14+
15+
### Why Not 60 FPS?
16+
17+
Screen capture APIs have inherent limitations:
18+
19+
1. **macOS `CGDisplayCreateImageForRect`**: ~50-70ms per capture
20+
2. **Windows GDI `GetPixel`**: Can be faster for single pixels, but still limited for 9x9 grids
21+
3. **Linux (Wayland) `grim`**: Process spawning overhead (~100ms)
22+
4. **Linux (X11) ImageMagick**: Similar to Wayland
23+
24+
### Comparison to Old Approach
25+
26+
| Metric | Old (Screenshot) | New (Rust Sampler) |
27+
| ------------------- | ------------------ | ------------------ |
28+
| **Initial capture** | 200-500ms | 50-70ms |
29+
| **Update rate** | 0 FPS (frozen) | 15-20 FPS (live) |
30+
| **Color accuracy** | Frozen in time | Live/continuous |
31+
| **Memory usage** | Full screen bitmap | 9x9 grid only |
32+
| **Responsiveness** | Poor (frozen) | Good (live) |
33+
34+
### Optimization Strategies
35+
36+
#### What We've Done
37+
38+
1. ✅ **Batch grid sampling**: Capture entire 9x9 grid in one screenshot instead of 81 separate captures
39+
2. ✅ **Realistic sample rate**: Set to 20 FPS instead of unrealistic 60 FPS
40+
3. ✅ **Error handling**: Gracefully handle capture failures without crashing
41+
42+
#### Future Optimizations
43+
44+
1. **Use ScreenCaptureKit (macOS 12.3+)**: New API with better performance
45+
- Requires macOS 12.3+
46+
- Can achieve 30-60 FPS
47+
- More complex implementation
48+
49+
2. **GPU-accelerated capture**: Use Metal/DirectX for sampling
50+
- Platform-specific
51+
- Significant development effort
52+
53+
3. **Adaptive sampling**: Lower rate when cursor not moving
54+
- Save CPU when idle
55+
- Burst to higher rate on movement
56+
57+
4. **Smaller capture region**: Only capture what's needed
58+
- Already implemented (9x9 grid)
59+
- Can't optimize much further
60+
61+
## User Experience Impact
62+
63+
### What Users Will Notice
64+
65+
**Positive:**
66+
67+
- ✅ Color picker shows **live colors** as you move
68+
- ✅ Magnifier updates smoothly at 15-20 FPS
69+
- ✅ More responsive than screenshot-based approach
70+
- ✅ Lower memory usage
71+
72+
**Limitations:**
73+
74+
- ⚠️ Not quite "buttery smooth" 60 FPS
75+
- ⚠️ Slight lag when moving cursor quickly (50-70ms latency)
76+
- ⚠️ This is a limitation of screen capture APIs, not our implementation
77+
78+
### Comparison to System Color Pickers
79+
80+
- **macOS Digital Color Meter**: Also ~15-20 FPS (same API limitations)
81+
- **GIMP Color Picker**: Similar performance
82+
- **Professional tools** (e.g., Photoshop): Use similar APIs, similar performance
83+
84+
## Performance by Platform
85+
86+
### macOS
87+
88+
- **Current**: 15-20 FPS (50-70ms/frame)
89+
- **Theoretical max** (with ScreenCaptureKit): 30-60 FPS
90+
- **Bottleneck**: CGDisplay API
91+
92+
### Windows
93+
94+
- **Expected**: 20-30 FPS
95+
- **Bottleneck**: GDI GetPixel API for grid sampling
96+
- **Note**: Not yet tested in production
97+
98+
### Linux (Wayland)
99+
100+
- **Expected**: 10-15 FPS (cached at 100ms intervals)
101+
- **Bottleneck**: External `grim` tool spawning
102+
- **Note**: Caching helps reduce overhead
103+
104+
### Linux (X11)
105+
106+
- **Expected**: 10-20 FPS
107+
- **Bottleneck**: ImageMagick/scrot tool spawning
108+
- **Note**: Similar to Wayland
109+
110+
## Recommendations
111+
112+
### For Users
113+
114+
1. **This is normal**: 15-20 FPS is expected and good for a color picker
115+
2. **Better than alternatives**: Most color pickers have similar or worse performance
116+
3. **Smooth enough**: Human perception doesn't need 60 FPS for color picking
117+
118+
### For Developers
119+
120+
1. **Don't increase sample rate above 20 FPS**: Wastes CPU with no benefit
121+
2. **Consider ScreenCaptureKit** for macOS: Would give true 60 FPS
122+
3. **Profile before optimizing**: Current performance is acceptable
123+
4. **Focus on UX**: Smooth animations matter more than raw FPS
124+
125+
## Measuring Performance
126+
127+
The Rust sampler logs performance stats to stderr every 60 frames:
128+
129+
```
130+
Sampling at 16.2 FPS (target: 20 FPS)
131+
Warning: frame took 61ms (target: 50ms)
132+
```
133+
134+
To see these stats during development:
135+
136+
1. Run `pnpm start:electron`
137+
2. Check the terminal output
138+
3. Look for "Sampling at X FPS" messages
139+
140+
## Known Issues
141+
142+
### macOS Retina Displays
143+
144+
On Retina displays, the scale factor affects capture performance:
145+
146+
- 2x Retina: Slightly slower (~10-20% overhead)
147+
- Solution: Already accounted for in implementation
148+
149+
### Multiple Displays
150+
151+
Capturing near display edges can be slower:
152+
153+
- Affects: macOS, Windows, Linux
154+
- Mitigation: Use fallback gray color for out-of-bounds pixels
155+
156+
### CPU Usage
157+
158+
Screen capture is CPU-intensive:
159+
160+
- **Idle**: ~5-10% CPU (when color picker not active)
161+
- **Active**: ~20-30% CPU (during sampling)
162+
- **Normal**: This is expected for continuous screen capture
163+
164+
## Conclusion
165+
166+
The Rust-based sampler provides **significant improvements** over the screenshot-based approach:
167+
168+
- ✅ **15-20 FPS** continuous sampling (vs 0 FPS frozen)
169+
- ✅ **Live color updates** (vs frozen snapshot)
170+
- ✅ **Lower memory** usage (grid only vs full screen)
171+
- ✅ **Better UX** (responsive vs laggy)
172+
173+
While not hitting the theoretical 60 FPS target, this is a **fundamental limitation of screen capture APIs** on all platforms. Future optimizations (like ScreenCaptureKit on macOS) could improve this, but the current performance is **good enough for production use** and **better than most alternatives**.

0 commit comments

Comments
 (0)