Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Win32] Combine duplicate initialization logic for image from providers #1886

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Mar 7, 2025

Images based on ImageFileNameProvider and ImageDataProvider are initialized with data based on the default zoom and can later on be reinitialized with data for others zooms when getImageData() is called. At both places, the same functionality is implemented twice.

This change makes the initial creation of image data use the already existing implementation for reinitializing it for another zoom. It also unifies the previous two implementations and removes obsolete calls to init() which are just required once for starting resource tracking.

Required to keep changes in #1828 simple.

Copy link
Contributor

github-actions bot commented Mar 7, 2025

Test Results

   505 files  ±0     505 suites  ±0   7m 50s ⏱️ -54s
 4 304 tests ±0   4 292 ✅ +1   11 💤 ±0  1 ❌  - 1 
16 560 runs  ±0  16 453 ✅ +1  106 💤 ±0  1 ❌  - 1 

For more details on these failures, see this check.

Results for commit 14056ad. ± Comparison against base commit d25bdc3.

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare marked this pull request as ready for review March 7, 2025 11:41
@HeikoKlare
Copy link
Contributor Author

@akoch-yatta can you please have a look at this and check whether you see any reasons against this simplification?

@HeikoKlare HeikoKlare force-pushed the win32-image-unify-duplicate-initialization branch 2 times, most recently from 6787b44 to 1033752 Compare March 7, 2025 15:34
Copy link
Contributor

@akoch-yatta akoch-yatta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look solid to me. Unifying the call from the constructor with the almost equal ones for rescaling makes sense.

Images based on ImageFileNameProvider and ImageDataProvider are
initialized with data based on the default zoom and can later on be
reinitialized with data for others zooms when `getImageData()` is
called. At both places, the same functionality is implemented twice.

This change makes the initial creation of image data use the already
existing implementation for reinitializing it for another zoom. It also
unifies the previous two implementations and removes obsolete calls to
init() which are just required once for starting resource tracking.
@HeikoKlare HeikoKlare force-pushed the win32-image-unify-duplicate-initialization branch from 1033752 to 14056ad Compare March 8, 2025 11:33
@HeikoKlare
Copy link
Contributor Author

Failing test is unrelated and already documented: #1843

@HeikoKlare HeikoKlare merged commit 31a54c4 into eclipse-platform:master Mar 8, 2025
12 of 14 checks passed
@HeikoKlare HeikoKlare deleted the win32-image-unify-duplicate-initialization branch March 8, 2025 11:43
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