Skip to content

[Win32] Simplify and fix Region class#3224

Draft
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
HeikoKlare:cleanup/simplify-win32-region
Draft

[Win32] Simplify and fix Region class#3224
HeikoKlare wants to merge 1 commit intoeclipse-platform:masterfrom
HeikoKlare:cleanup/simplify-win32-region

Conversation

@HeikoKlare
Copy link
Copy Markdown
Contributor

Summary

This change cleans up the win32 Region.java implementation by removing duplication, fixing a formatting bug, and applying idiomatic Java patterns throughout.

Bug fix

  • toString() produced malformed output: The method opened a { but never closed it, producing strings like Region {handle(zoom:100) instead of Region {handle(zoom:100)}. This made debugging harder and was inconsistent with the disposed-region case which does close its braces.

Simplifications

  • getRegionHandle(int): Replaced the manual containsKey / put / get triple with Map.computeIfAbsent, which is the idiomatic Java way to lazily populate a map and avoids two redundant lookups.

  • getBounds(): A block lambda containing only a single return was converted to an expression lambda.

  • isEmpty(): The RECT object was allocated before the lambda, even though it is only needed inside it. Moved it inside the lambda to keep allocation and use co-located and to avoid capturing a mutable object across a closure boundary unnecessarily.

Eliminated code duplication

  • OperationWithRectangle: The three methods addInPixels, subtractInPixels, and intersectInPixels were virtually identical — each created a rect region, called CombineRgn with a different mode constant, then deleted the region. These were replaced by a single combineWithRectInPixels(handle, x, y, w, h, mode) helper, reducing ~15 lines to ~5.

  • OperationWithArray: The same pattern applied to addInPixels and subtractInPixels, collapsed into combineWithPolyInPixels(handle, pointArray, mode).

  • OperationWithRegion: Three block lambdas each containing a single return OS.CombineRgn(...) call were converted to expression lambdas, removing the syntactic noise.

Access modifiers

  • Removed redundant public from the constructors of OperationWithArray and OperationWithPoint. Both classes are declared private static, so public on their constructors has no semantic effect and is misleading.

Test plan

  • Existing Region-related JUnit tests pass without modification
  • Verify toString() output now includes the closing } for non-disposed regions
  • Smoke-test region operations (add, subtract, intersect, translate) on Windows

- Fix toString() missing closing brace (output was malformed)
- Use computeIfAbsent() in getRegionHandle() instead of manual
  containsKey/put/get
- Move RECT allocation inside isEmpty() lambda where it belongs
- Replace getBounds() block lambda with expression lambda
- Eliminate duplicate create/combine/delete patterns in
  OperationWithRectangle (3 methods -> combineWithRectInPixels)
  and OperationWithArray (2 methods -> combineWithPolyInPixels)
- Convert OperationWithRegion block lambdas to expression lambdas
- Remove redundant public modifiers on constructors of private
  inner classes OperationWithArray and OperationWithPoint
@HeikoKlare HeikoKlare changed the title Simplify and fix win32 Region.java [Win32] Simplify and fix Region class Apr 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Test Results (win32)

   28 files  ±0     28 suites  ±0   4m 27s ⏱️ -9s
4 666 tests ±0  4 593 ✅ ±0  73 💤 ±0  0 ❌ ±0 
1 200 runs  ±0  1 176 ✅ ±0  24 💤 ±0  0 ❌ ±0 

Results for commit 7ce8659. ± Comparison against base commit 188aed6.

@vogella
Copy link
Copy Markdown
Contributor

vogella commented Apr 11, 2026

I did not check in the code (sorry)but would that cleanup also make sense for Linux ans Mac Implementation or why is this "only" done for windows?

@HeikoKlare
Copy link
Copy Markdown
Contributor Author

The Resource implementations (Region, Transform, Image etc.) are significantly different between Windows and macOS/Linux (due to Windows requiring handles for multiple zooms for proper multi-monitor scaling support, which is non-existent in the limited HiDPI support of macOS and Linux). So, a cleanup may be performed on the Linux and macOS implementations as well, but the result would be significantly different. I chose to apply this to the Windows implementations (first) as those have been recently revised (for monitor-specific scaling).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants