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

Wrong units comparison when deciding on initial window size #56

Closed
maximbaz opened this issue Sep 9, 2020 · 7 comments · Fixed by #83
Closed

Wrong units comparison when deciding on initial window size #56

maximbaz opened this issue Sep 9, 2020 · 7 comments · Fixed by #83
Labels
bug Something isn't working

Comments

@maximbaz
Copy link

maximbaz commented Sep 9, 2020

Hi again 🙂

I'm on HiDPI screen (scaling factor 2.5), and I noticed that sometimes the window size is completely off, like the height of swappy window would be two times bigger than the height of my screen.

Turns out this is caused by comparing different units when deciding on window size.

swappy/src/application.c

Lines 555 to 569 in f3a4ae9

double threshold = 0.75;
double scaling = 1.0;
if (state->geometry->width > workarea.width * threshold) {
scaling = workarea.width * threshold / state->geometry->width;
} else if (state->geometry->height > workarea.height * threshold) {
scaling = workarea.height * threshold / state->geometry->height;
}
state->window->width = state->geometry->width * scaling;
state->window->height = state->geometry->height * scaling;
g_info("size of monitor at window: %ux%u", workarea.width, workarea.height);
g_info("size of window to render: %ux%u", state->window->width,
state->window->height);

My screen is 3840x2400px. I'm opening a 1237x2397px image with swappy -f image.png and here's what is logged:

** INFO: 23:26:13.993: size of image: 1237x2397
** INFO: 23:26:14.024: size of monitor at window: 1536x960
** INFO: 23:26:14.024: size of window to render: 1152x2232

Size of image is used as-is, while size of monitor is divided by scale value of 2.5, so comparing those values is wrong.

As you can imagine, swappy window is now 2232*2.5=5580px in height!

I wasn't sure which way is proper to fix the issue, but I hope this gives you enough information to come up with a quick patch.

Thanks!

P.S. Have I already mentioned how awesome your tool is? It is! 😄
P.P.S. Excited about your rust refactoring branch 😉

@jtheoof
Copy link
Owner

jtheoof commented Oct 14, 2020

Thanks I think the logic for window size compute need a little bit more love.

The rust branch is really really WIP, I didn't do much on it during summer.

@jtheoof jtheoof added the bug Something isn't working label Oct 14, 2020
jtheoof added a commit that referenced this issue Feb 15, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes #56
@jtheoof
Copy link
Owner

jtheoof commented Feb 15, 2021

@maximbaz I reworked the initialization of the window and cairo surfaces.

It should fix this issue but might make #54 more perceivable.

I've tried on various image sizes (big heights, big widths) and scaled monitor. The window seems to have the proper size now.

Can you try to build the fix/high-dpi branch? And let me know if this fix is preferable over the lowered image quality for scaled displays.

@maximbaz
Copy link
Author

Thanks for working on this!

It seems I am still able to reproduce the issue 😞

Here's the image I'm using as a test (archived to make sure github doesn't do anything to it): image.zip

Here are the logs:

** INFO: 10:30:02.209: size of monitor at window: 1536x960
** INFO: 10:30:02.209: size of image: 1382x2400
** INFO: 10:30:02.209: maxium size allowed for window: 1152x720
** INFO: 10:30:02.209: size of window to render: 1152x2000
(swappy:2711593): dconf-DEBUG: 10:30:02.210: sync
** (swappy:2711593): DEBUG: 10:30:02.213: received configure_event callback
** INFO: 10:30:02.213: size of drawing area surface: 3456x6000
** INFO: 10:30:02.213: image scaled on x,y: 0.833575,0.833333
** INFO: 10:30:02.225: size of area to render: 1152x2000

And here's how it actually looks on my screen, you can see only maybe 1/3 of the image fits the screen and everything is oversized...

Swappshot Mon Feb 15 10:32:42 2021

This part is probably wrong: size of drawing area surface: 3456x6000

My screen resolution is 3840x2400 with scale of 2.5

jtheoof added a commit that referenced this issue Feb 15, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes #56
@jtheoof
Copy link
Owner

jtheoof commented Feb 15, 2021

I think I've got it 🤞

Can you try to git fetch --all && git reset --hard upstream/fix/high-dpi (assuming this repo is upstream).

@maximbaz
Copy link
Author

It's... different now 🙂

The window is not oversized, it fits the screen completely... but you are right, #54 is much more perceivable now 😞 Could you explain why is this the case, if you know already?

Also interestingly enough it is not 1:1 with reality, I expected to see the image inside swappy scaled exactly as reality, but here is me moving swappy right under waybar and you can see that stuff inside swappy is smaller:

image

And another interesting observation is that if I open an image with swappy and click "save", the saved image will be different in size!

$ file stripe2.png
stripe2.png: PNG image data, 1382 x 2400, 8-bit/color RGB, non-interlaced

$ file /tmp/screenshots/swappy-20210215_191630.png
/tmp/screenshots/swappy-20210215_191630.png: PNG image data, 1242 x 2160, 8-bit/color RGBA, non-interlaced

@jtheoof
Copy link
Owner

jtheoof commented Feb 15, 2021

All 3 issues you are mentioning are all related to #54.

Now the window (or GtkDrawingArea) in which the cairo buffer is rendered is scaled properly if the image you are trying to use is bigger than 75% of monitor size (width and/or height). So no more out of proportion windows. This was very likely to happen on scaled displays as you can see in your logs.

However because the final image (saved or copied) is taken from that GtkDrawingArea, then any scaling that might have taken place (if one of the threshold is reached) will also occur on that final image.

So in the end swappy lowers the resolution of the rendered buffer. This is not great and I need to find a clever way to fix that without re-writing the rendering pipeline.

It looks like this particular issue is fixed, I will probably merge it to master soon and see what I can do for #54.

@maximbaz
Copy link
Author

Sounds good, thank you!

jtheoof added a commit that referenced this issue Feb 15, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes #56
jtheoof added a commit that referenced this issue Feb 15, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes #56
jtheoof added a commit that referenced this issue Feb 15, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes #56
lelgenio pushed a commit to lelgenio/swappy that referenced this issue Feb 21, 2021
Compute maximum width and height of area container based on monitor size
and fix image ratio based on appropriate values.

Note that this will lower the resolution of the final image.

Closes jtheoof#56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants