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

Adapt double buffering use cases for multi-zoom #2722

Merged

Conversation

akoch-yatta
Copy link
Contributor

@akoch-yatta akoch-yatta commented Jan 17, 2025

This PR replaces three use cases where GC together with Images where used to draw double buffered images. With new multi-zoom-setting in Windows this leads to multiple issues like:

  • destructive scaling, e.g. up- or downscaling of the pixel data
  • fragments, when buffer image and second buffer image use different zoom

The changes utilize the newly added ImageGcDrawer to provide a dynamic callback to draw on a correctly initialized GC on demand.

Most important changes are:

  • cleanup of the buffer image on zoom change
  • Always do a full draw when a new variant is requested

How to test

  1. You need to use the branch from Callback for dynamic drawing with a GC on images eclipse.platform.swt#1734 for SWT.

  2. Either make sure you have the the experimental monitor specific flag enabled or use -Dswt.autoScale.updateOnRuntime=true -Dswt.autoScale=quarter as additional VM arguments

  3. Have at least two monitors with different zoom (e.g. one with 100, one with 150), detach a view and move it to the second monitor. Then you should be able to create fragments or other glitches in the text editors e.g. by just resizing it -> resizing always retriggers a full re-creation.

Example for OverviewRuler and LineNumberRulerColumn

If you have a monitor setup with 200% and 100%, have both (main view and detached view) on 200%, move one to the 100% monitor and resize the view remaining on 200%. You will see:
Before it blurry scales the 100% variant of the image and blurry scales the line number
Screenshot 2025-01-20 110042
After it correctly fetches the 200% variant of the image and the line numbers are redrawn sharply
Screenshot 2025-01-20 105208

Example for not refreshed line numbers

Use the same setup as above: 200% and 100% monitor, have both (main view and detached view) on 200%, move one to the 100% monitor and resize the view remaining on 200%. You should see the line number not correctly refreshed on scrolling though a file anymore

Important: As it is quite visible I want to mention, that the DefaultRangeIndicator will still looks glitchy like
image
in some scenarios. This is an additional issue related to ImageData and the destructive scaling of ImageData and will be fixed in a separate PR. This is a pre-existing issue, but the monitor aware scaling makes it way more obvious.

Copy link
Contributor

github-actions bot commented Jan 17, 2025

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 30m 23s ⏱️ + 1m 13s
 7 715 tests ±0   7 487 ✅ +1  228 💤 ±0  0 ❌  - 1 
24 306 runs  ±0  23 557 ✅ +1  749 💤 ±0  0 ❌  - 1 

Results for commit 57666c4. ± Comparison against base commit 8029b47.

♻️ This comment has been updated with latest results.

@HeikoKlare
Copy link
Contributor

Some minor additions on the PR:

  • Testing the changes without the experimental preference for monitor-specific scaling on Windows, the behavior should be the exact same as before the PR. The mentioned remaining glitches will only occur when having that experimental feature enabled.
  • Only derivation from the point above is the appearance on GTK when changing fractional scaling settings of the used monitors while running an Eclipse application. In this case, one should experience an improvement of the apperance of the line number ruler, see Callback for dynamic drawing with a GC on images eclipse.platform.swt#1734 (review)

This is how the improvement looks on GTK when switching from 100% to 150% scaling:

Before the changes

image

After the changes

image

@akoch-yatta akoch-yatta marked this pull request as ready for review January 20, 2025 08:10
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

The change looks good to me. I have tested it on all platforms together with eclipse-platform/eclipse.platform.swt#1734 and found no issues, but I will also test again today with the final state of the merged SWT changes.

I only have a minor comment on a potential code simplification.

@akoch-yatta akoch-yatta force-pushed the use-imagegcdrawer-for-columns branch 3 times, most recently from fc3aee2 to 6284318 Compare January 20, 2025 09:31
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

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

Thank you for adding the use cases to test @akoch-yatta .

I tested (in Windows) and found no regressions and also that the 2 mentioned use cases were fixed by this PR. I couldn't find any more glitches,

@akoch-yatta akoch-yatta force-pushed the use-imagegcdrawer-for-columns branch from 448fe66 to 6ab9306 Compare January 20, 2025 13:14
@akoch-yatta akoch-yatta force-pushed the use-imagegcdrawer-for-columns branch from 6ab9306 to dd3939f Compare January 20, 2025 14:57
This commit replaces three use cases where GC together with Images where
used to draw double buffered images. With new multi-zoom-setting in
Windows this leads to desctructive scaling and fragments caused by the
double buffering of Images/GCs created with different zoom. The changes
utilize the newly added ImageGcDrawer to provide a dynamic callback to
draw on a correctly initialized GC on demand.
@akoch-yatta akoch-yatta force-pushed the use-imagegcdrawer-for-columns branch from dd3939f to 57666c4 Compare January 20, 2025 15:00
@HeikoKlare HeikoKlare merged commit 3eaaeb6 into eclipse-platform:master Jan 20, 2025
17 checks passed
@HeikoKlare HeikoKlare deleted the use-imagegcdrawer-for-columns branch January 20, 2025 16:31
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.

UI Glitches in multi zoom environments for line numbers
3 participants