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

Display cover art image with the correct size on multi-monitor setups #2915

Merged
merged 2 commits into from Jul 21, 2018

Conversation

Projects
None yet
2 participants
@frestr
Member

frestr commented Jul 19, 2018

When clicking on the cover art of the playing song, the image is enlarged and fills approximately half of the width and height of the monitor. This seems to be the intended behavior.

On multi-monitor setups, the image does not necessarily enlarge in the same way. I have multiple monitors with varying sizes and orientations, and the image always fills to the edge of the current monitor. The reason for this is that the dimensions of Gdk.Screen (which combines all monitors into one big (virtual) one) are used when scaling the image.

A better way is to use the current monitor as a base when calculating the desired width and height of the image, which is what this commit does.

Leaving the patch here in case someone wants to have a say in this.

win = Gdk.Window.at_pointer()
disp = Gdk.Display.get_default()
if win[0] and disp:
mon = disp.get_monitor_at_window(win[0])

This comment has been minimized.

@lazka

lazka Jul 20, 2018

Member

this API is gtk 3.22+

This comment has been minimized.

@frestr

frestr Jul 20, 2018

Member

Ah, right. I'll make it compatible with older versions too.

@lazka

This comment has been minimized.

Member

lazka commented Jul 20, 2018

Any idea re the test failures?

@frestr

This comment has been minimized.

Member

frestr commented Jul 20, 2018

It seems like the docker container running the tests is being SIGTERM'ed (exit code 143). I initially though it may have had something to do with the CircleCI outage, but it's strange that it's still happening if that's the case.

@lazka

This comment has been minimized.

Member

lazka commented Jul 20, 2018

Yeah, had the same problem/error with #2846 and I had to revert. Maybe something xvfb related.

@frestr

This comment has been minimized.

Member

frestr commented Jul 20, 2018

Argh, still failing. I'll try to investigate.

frestr added some commits Jul 19, 2018

qltk/cover: Use active monitor as base when scaling
As opposed to the Gdk.Screen object, which amounts to a giant screen in
multi-monitor setups.
qltk/cover: Use old API when calculating monitor size with gtk<3.22
New API methods were introduced in 3.22, and the old ones deprecated.

@frestr frestr force-pushed the frestr:cover_size branch from 173432b to b520530 Jul 20, 2018

@frestr

This comment has been minimized.

Member

frestr commented Jul 20, 2018

Ok, so the problem was simply that my fork's master was updated right after the commit that made the tests fail. Fetching upstream and rebasing fixed it.

@lazka lazka merged commit 2b846e3 into quodlibet:master Jul 21, 2018

6 checks passed

ci/circleci: job.fedora28 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu16.04 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu18.04 Your tests passed on CircleCI!
Details
ci/circleci: job.win32 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@frestr frestr deleted the frestr:cover_size branch Jul 21, 2018

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