-
Notifications
You must be signed in to change notification settings - Fork 184
[GTK] Fix scaling of image copies and stylings #2653
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
base: master
Are you sure you want to change the base?
Conversation
@akurtakov since you are the GTK expert and also recently cleaned up autoscaling stuff that, if I am not mistaken, was necessary before Cairo autoscaling, maybe you want to have a look at this fix for image scaling? |
Test Results 115 files 115 suites 12m 57s ⏱️ Results for commit 5f7b1d2. ♻️ This comment has been updated with latest results. |
double scaleFactor = DPIUtil.getDeviceZoom() / 100f; | ||
Cairo.cairo_surface_set_device_scale(surface, scaleFactor, scaleFactor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is setting the scale still necessary? I don't get what the purpose is when the surface already has the correct size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is necessary. Otherwise it will look like this:
In my understanding, you need to do two things (which are the two places I adapted):
- Give the correct size in pixels to the surface; by now we used the size in points, which led to have the expected size at 200% zoom
- Tell Cairo at what scale that surface is used to ensure that coordinates are properly transformed; this is basically the same we have to do in SWT with point/pixel conversion. If you leave that out, you have a surface of double the size (when device zoom is 200) but since you treat all coordinates as 100%, it will paint at half scale to the top-left area of the surface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still not able to understand a slight detail on how the images were drawn fully (although blurrily scaled) before, as the height and width of the surface was in points I would expect to the surface to have only data in half width and height or the image to be partially cut off, when the zoom is 200%
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because the surface scale was usually set to 1 as currentDeviceZoom
and DPIUtil.getDeviceZoom()
were equal. I don't know why the check was there at all.
I tried running with a snippet to draw a image and its copy. I can see a copied image would be cut off to only draw the top left quarter with the current state in master, which is also fixed by changes in this PR. Could also use this snippet to test the changes here. package org.eclipse.swt.snippets;
import org.eclipse.swt.*;
import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;
public class ImageDrawExample {
public static void main(String[] args) {
Display display = new Display();
Shell shell = new Shell(display);
shell.setText("Image Enabled/Disabled Drawing");
shell.setSize(600, 600);
Image enabledImage = new Image(display, "src/org/eclipse/swt/snippets/eclipse.svg");
Image copyImage = new Image(display, enabledImage, SWT.IMAGE_COPY);
shell.addListener(SWT.Paint, e -> {
e.gc.drawText("Enabled Image:", 10, 10);
e.gc.drawImage(enabledImage, 10, 30);
e.gc.drawText("Disabled Image:", 300, 10);
e.gc.drawImage(copyImage, 300, 30);
});
shell.open();
while (!shell.isDisposed()) {
if (!display.readAndDispatch()) display.sleep();
}
enabledImage.dispose();
copyImage.dispose();
display.dispose();
}
} I have also tested the changes with the runtime workspace and could observe the original mentioned issue is resolved. |
The image copy and styling operation via the constructor Image(Device, Image, int) does currently not properly take into account a potential scaling of the original image. The surface for the new image is always created with the size in points, i.e., according to a 100% scaling. When the image is used at 200% it will thus appear blurry. This change adapts the image copy and styling constructor to properly take the scaling of the original image into account. It make the constructor create a surface with a proper size and scaling.
62f6c2b
to
5f7b1d2
Compare
The image copy and styling operation via the constructor Image(Device, Image, int) does currently not properly take into account a potential scaling of the original image. The surface for the new image is always created with the size in points, i.e., according to a 100% scaling. When the image is used at 200% it will thus appear blurry.
This change adapts the image copy and styling constructor to properly take the scaling of the original image into account. It make the constructor create a surface with a proper size and scaling.
This will also resolve the issues discussed in:
Before
After